My latest rookie SQL mistake...

By Steve Endow

I just discovered a fun mistake that I made in a SQL script.  It's a rookie mistake, but it's one of those somewhat novel mistakes that I think is easily missed in many projects.

I developed a Dynamics GP SOP Invoice import for a customer using .NET and eConnect.  It has been in use for over 3 years and working great, but recently they finally had a scenario where they uncovered the latent bug.

After reviewing my code and looking at the data that triggered the bug, I found that I had a design flaw in a SQL statement.  The flaw wasn't discovered during testing because I never anticipated a specific use case and boundary condition, so I never tested the scenario, and it took over 3 years for the customer to encounter it.

The customer is unique in that they will import an invoice, such as invoice number 123456, that relates to contract number 123456.  Then a few days later they will need to make an adjustment to the contract, so they will issue a related invoice to add services to the contract.  To help track the related transaction, the new invoice is imported into GP with a suffix on the invoice number, such as 123456-1.  A few days later, they will issue a credit memo to decrease the contract amount, and that CM will be imported as document number 123456-2, etc.  These numeric suffixes are added to the document number by the eConnect import.

Last week, the customer emailed me with a problem.  They were getting this eConnect error:


Error Number = 2221  Stored Procedure= taSopLineIvcInsert  Error Description = Duplicate document number. If adding or updating lines to an existing document, UpdateIfExists must = 1 
Node Identifier Parameters: taSopLineIvcInsert SOPNUMBE = 123456-10  SOPTYPE = 3


Hmmm.  So it looks like invoice 123456-10 already exists in GP.  So why did the import think that the -10 suffix should be used?

This is the SQL that is being used to get the last document number suffix from GP.

declare @SOPNUMBE as varchar(15)

set @SOPNUMBE = '123456-%'

SELECT ISNULL(MAX(SOPNUMBE), '') AS SOPNUMBE FROM

(SELECT SOPNUMBE FROM SOP10100 WHERE SOPNUMBE LIKE @SOPNUMBE

UNION SELECT SOPNUMBE FROM SOP30200 WHERE SOPNUMBE LIKE @SOPNUMBE) as dtSOP


Do you see my mistake?

It's not a syntax issue or a typo--the SQL will run just fine.  The mistake is a design flaw.

Do you see it yet?  (if it's not completely obvious, I'll feel better)

Here's a clue.  If there are 10 or more suffixes for a given contract, the query will always return 123456-9.  So if invoice 123456-10 already exists in GP, the query will still return a MAX value of 123456-9.

That clue probably makes it obvious that my mistake was using MAX directly on the SOPNUMBE field, which is a char field.

The T-SQL documentation doesn't explicitly discuss this use case, but if you've done much programming (and perhaps even just sorting data in Excel) you've probably seen this problem.  When sorting character data, numbers such as 10, 11, 12, 20, 30, etc. all sort before 9.  If the first digit is less than 9, anything from 10 to 100,000 will sort alphabetically before the number 9.

So, when the MAX(SOPNUMBER) function is called, it looks at the invoices from 123456-1 to 123456-10 and declares 123456-9 as the MAX.

Using the MAX function on a char field was my rookie mistake.  And not testing on invoices with more than 10 adjustments was the use case that I didn't think of during the testing process.

So how do I fix this design flaw?  Well, I need to find a way to convert the suffix to a numeric value so that I can find the numeric max value of all of the suffixes. 

There are probably several ways to accomplish this, but here's what I came up with.

SELECT ISNULL(MAX(SuffixValue), 0) AS Suffix FROM

(

SELECT CAST(SUBSTRING(SOPNUMBE, CHARINDEX('-', SOPNUMBE) + 1, LEN(SOPNUMBE) - CHARINDEX('-', SOPNUMBE)) AS int) AS SuffixValue

FROM MaxTest

WHERE SOPNUMBE LIKE '123450-%'

) AS dtSuffix

This version converts the suffix to an integer, and those results go into a derived table so that I can then use MAX.  It looks like this should work.

So I thought I would share this example of how a little mistake can easily be overlooked, resulting in a latent bug that can take years to manifest.  And a rookie mistake, no less!

Steve Endow is a Microsoft MVP in Los Angeles.  He is the owner of Precipio Services, which provides Dynamics GP integrations, customizations, and automation solutions.

You can also find him on Twitter, YouTube, and Google+

http://www.precipioservices.com