Tonmoy Saha
Tonmoy Saha

Reputation: 702

Getting null value in status field, can anyone explain what I am doing wrong?

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

Answers (2)

ErikE
ErikE

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

Siyual
Siyual

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

Related Questions