Close Menu

    CodeSOD: Join Us in this Query

    Today’s anonymous submitter worked for a “large, US-based, e-commerce company.” This particular company was, some time back, looking to save money, and like so many companies do, that meant hiring offshore contractors.

    Now, I want to stress, there’s certainly nothing magical about national borders which turns software engineers into incompetents. The reality is simply that contractors never have their client’s best interests at heart; they only want to be good enough to complete their contract. This gets multiplied by the contracting firm’s desire to maximize their profits by keeping their contractors as booked as possible. And it gets further multiplied by the remoteness and siloing of the interaction, especially across timezones. Often, the customer sends out requirements, and three months later gets a finished feature, with no more contact than that- and it never goes well.

    All that said, let’s look at some SQL Server code. It’s long, so we’ll take it in chunks.

    <span class="hljs-comment">-- ===============================================================================</span>
<span class="hljs-comment">-- Author     : Ignacius Ignoramus</span>
<span class="hljs-comment">-- Create date: 04-12-2020</span>
<span class="hljs-comment">-- Description:	SP of Getting Discrepancy of Allocation Reconciliation Snapshot</span>
<span class="hljs-comment">-- ===============================================================================</span>

    That the comment reinforces that this is an “SP”, aka stored procedure, is already not my favorite thing to see. The description is certainly made up of words, and I think I get the gist.

    <span class="hljs-keyword">ALTER</span> <span class="hljs-keyword">PROCEDURE</span> [dbo].[Discrepency]
	(
		<span class="hljs-variable">@startDate</span> DATETIME,
		<span class="hljs-variable">@endDate</span> DATETIME
	)
<span class="hljs-keyword">AS</span>

<span class="hljs-keyword">BEGIN</span>

    Nothing really to see here; it’s easy to see that we’re going to run a query for a date range. That’s fine and common.

    	<span class="hljs-keyword">DECLARE</span> <span class="hljs-variable">@tblReturn</span> <span class="hljs-keyword">TABLE</span>
	(
		intOrderItemId	   <span class="hljs-type">INT</span>
	)

    Hmm. T-SQL lets you define table variables, which are exactly what they sound like. It’s a local variable in this procedure, that acts like a table. You can insert/update/delete/query it. The vague name is a little sketch, and the fact that it holds only one field also makes me go “hmmm”, but this isn’t bad.

    	<span class="hljs-keyword">DECLARE</span> <span class="hljs-variable">@tblReturn1</span> <span class="hljs-keyword">TABLE</span>
	(
		intOrderItemId	   <span class="hljs-type">INT</span>
	)

    Uh oh.

    	<span class="hljs-keyword">DECLARE</span> <span class="hljs-variable">@tblReturn2</span> <span class="hljs-keyword">TABLE</span>
	(
		intOrderItemId	   <span class="hljs-type">INT</span>
	)

    Oh no.

    	<span class="hljs-keyword">DECLARE</span> <span class="hljs-variable">@tblReturn3</span> <span class="hljs-keyword">TABLE</span>
	(
		intOrderItemId	   <span class="hljs-type">INT</span>
	)

    Oh no no no.

    	<span class="hljs-keyword">DECLARE</span> <span class="hljs-variable">@tblReturn4</span> <span class="hljs-keyword">TABLE</span>
	(
		intOrderItemId	   <span class="hljs-type">INT</span>
	)

    This doesn’t bode well.

    So they’ve declared five variables called tblReturn, that all hold the same data structure.

    What happens next? This next block is gonna be long.

    	<span class="hljs-keyword">INSERT</span> <span class="hljs-keyword">INTO</span> <span class="hljs-variable">@tblReturn</span> <span class="hljs-comment">--(intOrderItemId) VALUES (@_ordersToBeAllocated)</span>

	<span class="hljs-comment">/* OrderItemsPlaced */</span>		

		<span class="hljs-keyword">select</span> 		
		intOrderItemId
		<span class="hljs-keyword">from</span> CompanyDatabase..Orders o
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..OrderItems oi <span class="hljs-keyword">on</span> oi.intOrderId <span class="hljs-operator">=</span> o.intOrderId
		<span class="hljs-keyword">where</span> o.dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>


		<span class="hljs-keyword">AND</span> intOrderItemId <span class="hljs-keyword">Not</span> <span class="hljs-keyword">In</span> 
		(

		<span class="hljs-comment">/* _itemsOnBackorder */</span>

		<span class="hljs-keyword">select</span> intOrderItemId			
		<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
		<span class="hljs-keyword">where</span> o.dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>
		<span class="hljs-keyword">and</span> oi.strstatus<span class="hljs-operator">=</span><span class="hljs-string">'backordered'</span> 
		)

		<span class="hljs-keyword">AND</span> intOrderItemId <span class="hljs-keyword">Not</span> <span class="hljs-keyword">In</span> 
		(

		<span class="hljs-comment">/* _itemsOnHold */</span>

		<span class="hljs-keyword">select</span> intOrderItemId			
		<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
		<span class="hljs-keyword">where</span> o.dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>
		<span class="hljs-keyword">and</span> o.strstatus<span class="hljs-operator">=</span><span class="hljs-string">'ONHOLD'</span>
		<span class="hljs-keyword">and</span> oi.strStatus <span class="hljs-operator"><></span> <span class="hljs-string">'BACKORDERED'</span> 
		)

		<span class="hljs-keyword">AND</span> intOrderItemId <span class="hljs-keyword">Not</span> <span class="hljs-keyword">In</span> 
		(

		<span class="hljs-comment">/* _itemsOnReview */</span>

		<span class="hljs-keyword">select</span>  intOrderItemId			
		<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
		<span class="hljs-keyword">where</span> o.dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span> 
		<span class="hljs-keyword">and</span> o.strstatus<span class="hljs-operator">=</span><span class="hljs-string">'REVIEW'</span> 
		<span class="hljs-keyword">and</span> oi.strStatus <span class="hljs-operator"><></span> <span class="hljs-string">'BACKORDERED'</span>
		)

		<span class="hljs-keyword">AND</span> intOrderItemId <span class="hljs-keyword">Not</span> <span class="hljs-keyword">In</span> 
		(

		<span class="hljs-comment">/*_itemsOnPending*/</span>

		<span class="hljs-keyword">select</span>  intOrderItemId			
		<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
		<span class="hljs-keyword">where</span> o.dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>
		<span class="hljs-keyword">and</span> o.strstatus<span class="hljs-operator">=</span><span class="hljs-string">'PENDING'</span>
		<span class="hljs-keyword">and</span> oi.strStatus <span class="hljs-operator"><></span> <span class="hljs-string">'BACKORDERED'</span>
		)

		<span class="hljs-keyword">AND</span> intOrderItemId <span class="hljs-keyword">Not</span> <span class="hljs-keyword">In</span> 
		(

		<span class="hljs-comment">/*_itemsCancelled */</span>

		<span class="hljs-keyword">select</span>  intOrderItemId			
		<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
		<span class="hljs-keyword">where</span> o.dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>
		<span class="hljs-keyword">and</span> oi.strstatus<span class="hljs-operator">=</span><span class="hljs-string">'CANCELLED'</span> 
		)

    We insert into @tblReturn the result of a query, and this query relies heavily on using a big pile of subqueries to decide if a record should be included in the output- but these subqueries all query the same tables as the root query. I’m fairly certain this could be a simple join with a pretty readable where clause, but I’m also not going to sit here and rewrite it right now, we’ve got a lot more query to look at.

    <span class="hljs-keyword">INSERT</span> <span class="hljs-keyword">INTO</span> <span class="hljs-variable">@tblReturn1</span>

		
		<span class="hljs-comment">/* _backOrderItemsReleased */</span>	

		<span class="hljs-keyword">select</span>  intOrderItemId			
		<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..orders o <span class="hljs-keyword">on</span> o.intorderid <span class="hljs-operator">=</span> oi.intorderid
		<span class="hljs-keyword">where</span> oi.intOrderItemid <span class="hljs-keyword">in</span> (
			  <span class="hljs-keyword">select</span> intRecordID 
			  <span class="hljs-keyword">from</span> CompanyDatabase..StatusChangeLog
			  <span class="hljs-keyword">where</span> strRecordType <span class="hljs-operator">=</span> <span class="hljs-string">'OrderItem'</span>
			  <span class="hljs-keyword">and</span> strOldStatus <span class="hljs-keyword">in</span> (<span class="hljs-string">'BACKORDERED'</span>)
			  <span class="hljs-keyword">and</span> strNewStatus <span class="hljs-keyword">in</span> (<span class="hljs-string">'NEW'</span>, <span class="hljs-string">'RECYCLED'</span>)
			  <span class="hljs-keyword">and</span> dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>  
		)
		<span class="hljs-keyword">and</span> o.dtmTimeStamp <span class="hljs-operator"><</span> <span class="hljs-variable">@startDate</span>
		

		<span class="hljs-keyword">UNION</span>
		(
			<span class="hljs-comment">/*_pendingHoldItemsReleased*/</span>

			<span class="hljs-keyword">select</span>  intOrderItemId					
			<span class="hljs-keyword">from</span> CompanyDatabase..OrderItems oi
			<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..orders o <span class="hljs-keyword">on</span> o.intorderid <span class="hljs-operator">=</span> oi.intorderid
			<span class="hljs-keyword">where</span> oi.intOrderID <span class="hljs-keyword">in</span> (
				  <span class="hljs-keyword">select</span> intRecordID 
				  <span class="hljs-keyword">from</span> CompanyDatabase..StatusChangeLog
				  <span class="hljs-keyword">where</span> strRecordType <span class="hljs-operator">=</span> <span class="hljs-string">'Order'</span>
				  <span class="hljs-keyword">and</span> strOldStatus <span class="hljs-keyword">in</span> (<span class="hljs-string">'REVIEW'</span>, <span class="hljs-string">'ONHOLD'</span>, <span class="hljs-string">'PENDING'</span>)
				  <span class="hljs-keyword">and</span> strNewStatus <span class="hljs-keyword">in</span> (<span class="hljs-string">'NEW'</span>, <span class="hljs-string">'PROCESSING'</span>)
				  <span class="hljs-keyword">and</span> dtmTimeStamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>  
			)
			<span class="hljs-keyword">and</span> o.dtmTimeStamp <span class="hljs-operator"><</span> <span class="hljs-variable">@startDate</span>
			
		)

		<span class="hljs-keyword">UNION</span>

		<span class="hljs-comment">/* _reallocationsowingtonostock */</span>	
		(
			<span class="hljs-keyword">select</span> oi.intOrderItemID				   	 
			<span class="hljs-keyword">from</span> CompanyDatabase.dbo.StatusChangeLog 
			<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase.dbo.OrderItems oi <span class="hljs-keyword">on</span> oi.intOrderItemID <span class="hljs-operator">=</span> CompanyDatabase.dbo.StatusChangeLog.intRecordID
			<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase.dbo.Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId  

			<span class="hljs-keyword">where</span> strOldStatus <span class="hljs-operator">=</span> <span class="hljs-string">'RECYCLED'</span> <span class="hljs-keyword">and</span> strNewStatus <span class="hljs-operator">=</span> <span class="hljs-string">'ALLOCATED'</span> 
			<span class="hljs-keyword">and</span> CompanyDatabase.dbo.StatusChangeLog.dtmTimestamp <span class="hljs-operator">></span> <span class="hljs-variable">@endDate</span> <span class="hljs-keyword">and</span> 
			strRecordType <span class="hljs-operator">=</span> <span class="hljs-string">'OrderItem'</span>
			<span class="hljs-keyword">and</span> intRecordId <span class="hljs-keyword">in</span> 
			(
			  <span class="hljs-keyword">select</span> intRecordId <span class="hljs-keyword">from</span> CompanyDatabase.dbo.StatusChangeLog 
			  <span class="hljs-keyword">where</span> strOldStatus <span class="hljs-operator">=</span> <span class="hljs-string">'ALLOCATED'</span> <span class="hljs-keyword">and</span> strNewStatus <span class="hljs-operator">=</span> <span class="hljs-string">'RECYCLED'</span> 
			  <span class="hljs-keyword">and</span> strRecordType <span class="hljs-operator">=</span> <span class="hljs-string">'OrderItem'</span>
			  <span class="hljs-keyword">and</span> CompanyDatabase.dbo.StatusChangeLog.dtmTimestamp <span class="hljs-keyword">between</span> <span class="hljs-variable">@startDate</span> <span class="hljs-keyword">and</span>  <span class="hljs-variable">@endDate</span>  
			)  
		)

    Okay, just some unions with more subquery filtering. More of the same. It’s the next one that makes this special.

    <span class="hljs-keyword">INSERT</span> <span class="hljs-keyword">INTO</span> <span class="hljs-variable">@tblReturn2</span>

	<span class="hljs-keyword">SELECT</span> intOrderItemId <span class="hljs-keyword">FROM</span> <span class="hljs-variable">@tblReturn</span> 
	
	<span class="hljs-keyword">UNION</span>

	<span class="hljs-keyword">SELECT</span> intOrderItemId <span class="hljs-keyword">FROM</span> <span class="hljs-variable">@tblReturn1</span>

    Ah, here’s the stuff. This is just bonkers. If the goal is to combine the results of these queries into a single table, you could just insert into one table the whole time.

    But we know that there are 5 of these tables, so why are we only going through the first two to combine them at this point?

        <span class="hljs-keyword">INSERT</span> <span class="hljs-keyword">INTO</span> <span class="hljs-variable">@tblReturn3</span>

		<span class="hljs-comment">/* _factoryAllocation*/</span>

		<span class="hljs-keyword">select</span> 
		oi.intOrderItemId                              
		<span class="hljs-keyword">from</span> CompanyDatabase..Shipments s 
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..ShipmentItems si <span class="hljs-keyword">on</span> si.intShipmentID <span class="hljs-operator">=</span> s.intShipmentID
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> Common.CompanyDatabase.Stores stores <span class="hljs-keyword">on</span> stores.intStoreID <span class="hljs-operator">=</span> s.intLocationID
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..OrderItems oi <span class="hljs-keyword">on</span> oi.intOrderItemId <span class="hljs-operator">=</span> si.intOrderItemId                                      
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> s.intOrderId  
		<span class="hljs-keyword">where</span> s.dtmTimestamp <span class="hljs-operator">>=</span> <span class="hljs-variable">@endDate</span>
		<span class="hljs-keyword">and</span> stores.strLocationType <span class="hljs-operator">=</span> <span class="hljs-string">'FACTORY'</span>
		
		<span class="hljs-keyword">UNION</span> 
		(
	 	  <span class="hljs-comment">/*_storeAllocations*/</span>

		<span class="hljs-keyword">select</span> oi.intOrderItemId                               
		<span class="hljs-keyword">from</span> CompanyDatabase..Shipments s 
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..ShipmentItems si <span class="hljs-keyword">on</span> si.intShipmentID <span class="hljs-operator">=</span> s.intShipmentID
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> Common.CompanyDatabase.Stores stores <span class="hljs-keyword">on</span> stores.intStoreID <span class="hljs-operator">=</span> s.intLocationID
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..OrderItems oi <span class="hljs-keyword">on</span> oi.intOrderItemId <span class="hljs-operator">=</span> si.intOrderItemId                                      
		<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase..Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> s.intOrderId
		<span class="hljs-keyword">where</span> s.dtmTimestamp <span class="hljs-operator">>=</span> <span class="hljs-variable">@endDate</span>
		<span class="hljs-keyword">and</span> stores.strLocationType <span class="hljs-operator"><></span> <span class="hljs-string">'FACTORY'</span>
		)

		<span class="hljs-keyword">UNION</span>
		(
		<span class="hljs-comment">/* _ordersWithAllocationProblems */</span>
    	
			<span class="hljs-keyword">select</span> oi.intOrderItemId
			<span class="hljs-keyword">from</span> CompanyDatabase.dbo.StatusChangeLog
			<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase.dbo.OrderItems oi <span class="hljs-keyword">on</span> oi.intOrderItemID <span class="hljs-operator">=</span> CompanyDatabase.dbo.StatusChangeLog.intRecordID
			<span class="hljs-keyword">inner</span> <span class="hljs-keyword">join</span> CompanyDatabase.dbo.Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
			<span class="hljs-keyword">where</span> strRecordType <span class="hljs-operator">=</span> <span class="hljs-string">'orderitem'</span>
			<span class="hljs-keyword">and</span> strNewStatus <span class="hljs-operator">=</span> <span class="hljs-string">'PROBLEM'</span>
			<span class="hljs-keyword">and</span> strOldStatus <span class="hljs-operator">=</span> <span class="hljs-string">'NEW'</span>
			<span class="hljs-keyword">and</span> CompanyDatabase.dbo.StatusChangeLog.dtmTimestamp <span class="hljs-operator">></span> <span class="hljs-variable">@endDate</span>
			<span class="hljs-keyword">and</span> o.dtmTimestamp <span class="hljs-operator"><</span> <span class="hljs-variable">@endDate</span>
		)

    Okay, @tblReturn3 is more of the same. Nothing more to really add.

    	 <span class="hljs-keyword">INSERT</span> <span class="hljs-keyword">INTO</span> <span class="hljs-variable">@tblReturn4</span>
	
	 <span class="hljs-keyword">SELECT</span> intOrderItemId <span class="hljs-keyword">FROM</span> <span class="hljs-variable">@tblReturn2</span> <span class="hljs-keyword">WHERE</span>
	 intOrderItemId <span class="hljs-keyword">NOT</span> <span class="hljs-keyword">IN</span>(<span class="hljs-keyword">SELECT</span> intOrderItemId <span class="hljs-keyword">FROM</span> <span class="hljs-variable">@tblReturn3</span> )

    Ooh, but here we see something a bit different- we’re taking the set difference between @tblReturn2 and @tblReturn3. This would almost make sense if there weren’t already set operations in T-SQL which would handle all of this.

    Which brings us, finally, to the last query in the whole thing:

    <span class="hljs-keyword">SELECT</span> 
	 o.intOrderId
	,oi.intOrderItemId
	,o.dtmDate
	,oi.strDescription
	,o.strFirstName <span class="hljs-operator">+</span> o.strLastName <span class="hljs-keyword">AS</span> <span class="hljs-string">'Name'</span>
	,o.strEmail
	,o.strBillingCountry
	,o.strShippingCountry
	<span class="hljs-keyword">FROM</span> CompanyDatabase.dbo.OrderItems oi
	<span class="hljs-keyword">INNER</span> <span class="hljs-keyword">JOIN</span> CompanyDatabase.dbo.Orders o <span class="hljs-keyword">on</span> o.intOrderId <span class="hljs-operator">=</span> oi.intOrderId
	<span class="hljs-keyword">WHERE</span> oi.intOrderItemId <span class="hljs-keyword">IN</span> (<span class="hljs-keyword">SELECT</span> intOrderItemId <span class="hljs-keyword">FROM</span> <span class="hljs-variable">@tblReturn4</span>)
<span class="hljs-keyword">END</span>

    At the end of all this, I’ve determined a few things.

    First, the developer responsible didn’t understand table variables. Second,they definitely didn’t understand joins. Third, they had no sense of the overall workflow of this query and just sorta fumbled through until they got results that the client said were okay.

    And somehow, this pile of trash made it through a code review by internal architects and got deployed to production, where it promptly became the worst performing query in their application. Correction: the worst performing query thus far.

    [Advertisement]
    Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

    Source: Read More 

    Related Posts

    Leave A Reply

    For security, use of Google's reCAPTCHA service is required which is subject to the Google Privacy Policy and Terms of Use.