Accepted... and refused



  • I've been asked to debug a stored procedure used in a POS application. The POS programmer complained that the procedure would work for new transactions, but not for existing transactions.

    Here is the code...

    ---8<---

    CREATE PROCEDURE dbo.proc_transaction_result @iTransactionNumber int, @bAccepted, @bRefused AS

    SET @iTransactionNumber = dbo.generateNewTransactionNumber(GETDATE())

    IF (SELECT COUNT(*) FROM tb_transaction WHERE iTransactionNumber = @iTransactionNumber) = 0
    INSERT tb_transaction (iTransactionNumber, bAccepted, bRefused) VALUES (@iTransactionNumber,@bAccepted, @bRefused)
    ELSE
    UPDATE tb_transaction SET iTransactionNumber = @iTransactionNumber, bAccepted = @bAccepted, bRefused = @bRefused

    RETURN 100

    --->8---

    The bAccepted and bRefused fields (both booleans) puzzled me, especially since the table included some records with both fields having the same value. In regard to this situation, the programmer told me that those fields were linked to checkboxes and that sometimes "the stupid users" would either select both Accepted and Refused, or select none. (Duh!)

    The new transaction number generation was obviously misplaced. A funny thing is that this mistake was protecting the system from a bigger bug (the UPDATE statement).

     



  • Heh heh,

    That's great.  I love that a database design bug is propigated right the way through to the user interface, and then inconsistent data is blamed on the users.

    Would have been ammusing if that UPDATE had ever been triggered. 



  • [quote user="SpoonMeiser"]

    Heh heh,

    That's great.  I love that a database design bug is propigated right the way through to the user interface, and then inconsistent data is blamed on the users.

    Would have been ammusing if that UPDATE had ever been triggered. 

    [/quote]

    I think it wouldn't run - the transaction id field is most likely the PRIMARY key thus forced UNIQUE.

    What puzzles me more - it seems each time the procedure is run it creates a new record in some table (it seems that's what the id generator does), even if no INSERT is needed.



  • [quote user="patrys"]

    I think it wouldn't run - the transaction id field is most likely the PRIMARY key thus forced UNIQUE.

    [/quote]

     

    With a stored procedure that messed up, it's entirely possible that the table definition is also messed up enough to lack a primary key.  "What do we need that for?  We're enforcing it in the application layer anyway!"

     

    [quote user="patrys"] 

    What puzzles me more - it seems each time the procedure is run it creates a new record in some table (it seems that's what the id generator does), even if no INSERT is needed.

    [/quote]

     

    Like someone else said, the ID generator is misplaced; it should be in some other procedure.

     



  • A bigger question is, does POS stand for Point of Sale or Piece of....nevermind.

    It's kind of scary if that system is live, and no one ever tested doing an update before in development.

    I've said it before on other sites and I'll say it again.  SQL should require a where clause on all update and delete queries, even if it's just "where all" or something equivalent.  I've never known a sql programmer, myself included, that hasn't been burned by forgetting.

    Yeah, it's one's own fault for forgetting, but it would be so simply to make it impossible to forget.



  • [quote user="SpoonMeiser"]

    Heh heh,

    That's great.  I love that a database design bug is propigated right the way through to the user interface, and then inconsistent data is blamed on the users.

    Would have been ammusing if that UPDATE had ever been triggered. 

    [/quote]

    What does that UPDATE do then? I don't really know anything about databases etc; my best guess is it just puts the wrong accepted and rejected values into the database. 



  • [quote user="Quincy5"]What does that UPDATE do then? I don't really know anything about databases etc; my best guess is it just puts the wrong accepted and rejected values into the database. 

    [/quote]

     

    No WHERE clause: would have updated the values in every record in the table

     

     



  • [quote user="president_ch0ice"]

    I've been asked to debug a stored procedure used in a POS application. The POS programmer complained that the procedure would work for new transactions, but not for existing transactions.

    Here is the code...

    ---8<---

    CREATE PROCEDURE dbo.proc_transaction_result @iTransactionNumber int, @bAccepted, @bRefused AS

    SET @iTransactionNumber = dbo.generateNewTransactionNumber(GETDATE())

    IF (SELECT COUNT(*) FROM tb_transaction WHERE iTransactionNumber = @iTransactionNumber) = 0
    INSERT tb_transaction (iTransactionNumber, bAccepted, bRefused) VALUES (@iTransactionNumber,@bAccepted, @bRefused)
    ELSE
    UPDATE tb_transaction SET iTransactionNumber = @iTransactionNumber, bAccepted = @bAccepted, bRefused = @bRefused

    RETURN 100

    --->8---

    The bAccepted and bRefused fields (both booleans) puzzled me, especially since the table included some records with both fields having the same value. In regard to this situation, the programmer told me that those fields were linked to checkboxes and that sometimes "the stupid users" would either select both Accepted and Refused, or select none. (Duh!)

    The new transaction number generation was obviously misplaced. A funny thing is that this mistake was protecting the system from a bigger bug (the UPDATE statement).

    [/quote]

    The best WTFs have all been taken, but no-one has yet pointed out that the procedure returns 100, which is the common SQL return value for record not found.



  • @Noisy Crow said:

    @Quincy5 said:
    What does that UPDATE do then? I don't really know anything about databases etc; my best guess is it just puts the wrong accepted and rejected values into the database.
    No WHERE clause: would have updated the values in every record in the table

    If "iTransactionNumber" is a PRIMARY KEY (and with a name like that, i'd expect it to be), shurely the UPDATE would cause a conflict and revert the transaction / cancel the command / otherwise abort, thus destroying no more than one row (the first row clobbered by the UPDATE)?



  • [quote user="Irrelevant"]

    If "iTransactionNumber" is a PRIMARY KEY (and with a name like that, i'd expect it to be), shurely the UPDATE would cause a conflict and revert the transaction / cancel the command / otherwise abort, thus destroying no more than one row (the first row clobbered by the UPDATE)?

    [/quote]

    Surely you don't expect someone who would write that UPDATE statement to be intelligent enough to actually have a real, constraint-enforced primary key, do you? :-)

    Ken
     



  • This is a correct assumption. No primary keys in the database. No index either (who needs indexes on a 5GB table?). I actually heard the programmer saying that "primary keys are slowing down the queries".

     

     



  • You have my deepest sympathy and condolences. :-(

     


Log in to reply