Reputation: 702
I want to status field based on case condition, but I am getting null.
Declare @Status NVarchar(20);
Set @Status = Case When Exists ( Select GunSerialNo
From dbo.ArmouryIssueGun
Where ModifiedOn != Null
And CreatedOn != Null ) Then 'In Field'
When Exists ( Select GunSerialNo
From dbo.ArmouryIssueGun
Where ModifiedOn = Null
And CreatedOn != Null ) Then 'In Armory'
End;
-- Insert statements for procedure here
Select (Select BranchName
From Branch
Where BranchId = gun.BranchId
) As BranchName
, gun.SerialNo As GunSerialNo
, gun.GunType
, gun.ModelNo
, gun.GunId
, Convert(Varchar(12), aig.CreatedOn, 103) IssueDate
, Substring(Convert(Varchar(20), aig.CreatedOn, 9), 13, 5) + ' ' + Substring(Convert(Varchar(30), aig.CreatedOn, 9), 25, 2) As IssueTime
, cl.LicenceHolderName As CarriedBy
, (Select TypeName
From dbo.CommonValues
Where ID = aig.Purpose
) As Purpose
, @Status As status
, Convert(Varchar(12), aig.ModifiedOn, 103) As CollectedDate
, Substring(Convert(Varchar(20), aig.ModifiedOn, 9), 13, 5) + ' ' + Substring(Convert(Varchar(30), aig.ModifiedOn, 9), 25, 2) As TimeIn
From dbo.CarryAndUseLicence cl
Join dbo.Branch b
On b.BranchId = cl.BranchId
Join dbo.Gun gun
On cl.GunSerialNo = gun.SerialNo
Join dbo.ArmouryIssueGun aig
On aig.StaffId = cl.StaffId;
Upvotes: 3
Views: 139
Reputation: 50271
Your subqueries have no correlating clause! All they're doing is querying to see if any one row in the queried table is not null.
If you are interested in a status for a particular GunSerialNo
you must query for that, for example:
SELECT
FROM dbo.ArmouryIssueGun
WHERE
ModifiedOn IS NOT NULL
AND CreatedOn IS NOT Null
AND GunSerialNo = @GunSerialNo -- A specific GunSerialNo
;
However, I suspect you're not interested in the status for a single item. Instead, you want to know the status for a bunch of rows. In that case, storing a single value in a @Status
variable will do you no good. You need to incorporate this into a query, and correlate the subquery with the outer query like so (see Status =
expression):
Select (Select BranchName
From Branch
Where BranchId = gun.BranchId
) As BranchName
, gun.SerialNo As GunSerialNo
, gun.GunType
, gun.ModelNo
, gun.GunId
, Convert(Varchar(12), aig.CreatedOn, 103) IssueDate
, Substring(Convert(Varchar(20), aig.CreatedOn, 9), 13, 5) + ' ' + Substring(Convert(Varchar(30), aig.CreatedOn, 9), 25, 2) As IssueTime
, cl.LicenceHolderName As CarriedBy
, (Select TypeName
From dbo.CommonValues
Where ID = aig.Purpose
) As Purpose
, Status =
Case When Exists (
Select *
From dbo.ArmouryIssueGun aig
Where
gun.SerialNo = aig.GunSerialNo // correlate to outer query
AND aig.ModifiedOn != Null
AND aig.CreatedOn != Null
) Then 'In Field'
When Exists (
Select *
From dbo.ArmouryIssueGun aig
Where
gun.SerialNo = aig.GunSerialNo // correlate to outer query
AND aig.ModifiedOn = Null
AND aig.CreatedOn != Null
) Then 'In Armory'
End
, Convert(Varchar(12), aig.ModifiedOn, 103) As CollectedDate
, Substring(Convert(Varchar(20), aig.ModifiedOn, 9), 13, 5) + ' ' + Substring(Convert(Varchar(30), aig.ModifiedOn, 9), 25, 2) As TimeIn
From dbo.CarryAndUseLicence cl
Join dbo.Branch b
On b.BranchId = cl.BranchId
Join dbo.Gun gun
On cl.GunSerialNo = gun.SerialNo
Join dbo.ArmouryIssueGun aig
On aig.StaffId = cl.StaffId;
Also, please note these additional comments that could help you:
When you compare with NULL
, it will not work to use =
or !=
or <>
. The answer will always be NULL
, which is both not false, and not true. Nothing you can do will make anything else occur. x.Column != NULL
will be treated as false, as will NOT (x.Column != NULL)
. You must do x.Column IS NULL
or x.Column IS NOT NULL
or NOT (x.Column IS NULL)
.
It seems like there is a serious data consistency error possible with the database design. Using ModifiedOn
as a proxy for whether a gun is checked out is completely unsafe. The design assumes a "happy path" where a gun cannot be modified until it is checked out (very likely to be violated) and that once it has been modified it cannot be still in the armory (such as, checked back in). Your system is potentially headed for serious problems if you don't rectify this immediately with some actual status columns or another mechanism to indicate the current status of the gun.
If the ArmouryIssueGun
table has a one-to-one relationship with each gun, then something is wrong, unless it is absolutely, always, 100% reliably true in every circumstance that each gun can only be issued a single time. That seems very unlikely. It could be that ArmouryIssueGun
really isn't a historical table but represents a portion of the Gun
table (like a partial class in C#), and thus is properly 1-to-(0 or) 1. In that case, my comment about using ModifiedOn
would definitely be true as a completely unreliable way to tell if the gun is checked out to the field or not.
If the relationship is one-to-many as I suspect (and as it should be), using what amounts to a historical log table to detect the current state of something is often not the best design. Mistakes can be made. Log entries can fail to be inserted. There should be one canonical place to look, in a single row for each GunSerialNo
. Using the historical table makes it possible for a later person who doesn't know the weird proxy dependency you've taken in this query to think that simply inserting another ArmoryIssueGun
row will take care of the problem, but your query assumes that just having a ModifiedOn
value proves it is currently in the field, not just in the field at some point in time in the past.
Column names should be the same in every table. You should not have one column named SerialNo
and another with the same meaning called GunSerialNo
.
I can see how the serial number of a gun makes a reasonably good natural/business key for your database. However, I question whether there are situations where it is not such a good key for all your tables. How many characters long is a serial number? An int uses only 4 bytes, but if your serial numbers are 20 characters, then each row will take 5 times as much space for the serial number. Keep in mind that clustered index columns are repeated in every non-clustered index. I don't know the size of your database, but if for example you were creating a database that served the Army of an entire country, there could theoretically be millions of entries! At that point, performance really starts to matter, and the space used by a column becomes significant.
Joining to the same table twice is suboptimal. You can change your above query like so to make the join occur only once, and additionally to allow the SELECT clause to even include columns from the found, most recent, row:
OUTER APPLY (
SELECT TOP 1
aig.*
FROM
dbo.ArmoryIssueGun aig
WHERE
gun.SerialNo = aig.GunSerialNo
ORDER BY
aig.ModifiedOn DESC
) aig
There is also no reason to do subqueries such as the one you've done for BranchName
. It clutters the query and is inconsistent. Instead, use JOINs consistently:
LEFT JOIN Branch b
ON gun.BranchId = b.BranchId
Then you can simply use b.BranchName
in the SELECT
clause, and can additionally pull other columns from the Branch
table. Using subqueries like this, in my experience, leads to certain thinking habits that yield suboptimally organized and hard-to-understand queries that sometimes even have negative performance implications (like if you wanted another column from the Branch
table, and you just threw in another subquery, you'd have needlessly doubled the cost of fetching that data).
Upvotes: 4
Reputation: 16917
Your problem is here:
Set @Status = Case When Exists ( Select GunSerialNo
From dbo.ArmouryIssueGun
Where ModifiedOn != Null
And CreatedOn != Null ) Then 'In Field'
When Exists ( Select GunSerialNo
From dbo.ArmouryIssueGun
Where ModifiedOn = Null
And CreatedOn != Null ) Then 'In Armory'
End;
Specifically these parts:
Where ModifiedOn != Null
And CreatedOn != Null
And
Where ModifiedOn = Null
And CreatedOn != Null
NULL
cannot be equal to anything. Not even to another NULL
. Thus, you cannot use =
and !=
to compare them. You need to use IS NULL
and IS NOT NULL
instead.
Try changing them to the following:
Where ModifiedOn Is Not Null
And CreatedOn Is Not Null
And
Where ModifiedOn Is Null
And CreatedOn Is Not Null
Upvotes: 4