DEV Community

Amit
Amit

Posted on

Our SQL Server loop kept writing the previous row's value

#database #sql #sqlserver

A loop that looks correct, passes review, and still writes the wrong value, because of two documented T-SQL behaviors.

Here's a loop that resolves a value for each item: read it from a primary table, fall back to a secondary one when the primary has nothing, write the result. It looks correct. On the wrong data it writes the wrong value, and the wrong value isn't random: each bad row gets the value belonging to the row processed just before it.

No error, no exception. The loop reads correctly top to bottom. The cause is two T-SQL behaviors that Microsoft documents and that are easy to misread:

  • A variable declared inside a loop is set to NULL once for the whole batch, not once per iteration.
  • SELECT @var = column leaves the variable alone when the query returns no rows.

Either one is survivable alone. Together they make a missing row reuse the last good answer.

What the code looks like 

WHILE (@cnt > 0)
BEGIN
    DECLARE @Value VARCHAR(100)
    SELECT @Item_Code = ITEM FROM @ItemList WHERE RowNo = @cnt

    SELECT @Value = isnull(p.ITEM_VAL, f.ITEM_VAL)
    FROM @Primary p
    LEFT JOIN @Fallback AS f ON p.ITEM_CODE = f.ITEM_CODE
    WHERE p.ITEM_CODE = @Item_Code
      AND p.ITEM_SEQ = (SELECT MAX(pmax.ITEM_SEQ) FROM @Primary pmax WHERE pmax.ITEM_CODE = @Item_Code)

    IF (@Value IS NULL) OR LTRIM(RTRIM(@Value)) = ''
    BEGIN
        SELECT TOP 1 @Value = ITEM_VAL FROM @Fallback WHERE ITEM_CODE = @Item_Code
    END

    UPDATE @Target SET ITEM_VAL = @Value WHERE ITEM_CODE = @Item_Code
    SET @cnt = @cnt - 1
END
Enter fullscreen mode Exit fullscreen mode

Read top to bottom, the intent is plain: resolve from the primary table, fall back to the secondary one, write the result. The DECLARE @Value inside the loop looks like it hands each pass a clean variable. That assumption is the trap.

A DECLARE inside a loop isn't a reset

In C++, Java, or C#, a variable declared inside a loop body is a fresh binding every pass. T-SQL doesn't do that. A local variable's scope is the whole batch, and it's initialized to NULL when it's declared, not at the point in the code where you wrote DECLARE.

So putting DECLARE @Value inside the WHILE is just decoration. There's one @Value for the whole run, set to NULL once, and every iteration after the first inherits whatever the previous one left in it. That's also why SQL Server doesn't complain about a double declaration even though the DECLARE line is reached on every pass; it isn't a statement that re-runs each time.

A quick proof:

DECLARE @i INT = 0
WHILE @i < 3
BEGIN
    DECLARE @x INT
    SET @x = ISNULL(@x, 0) + 1
    PRINT @x
    SET @i = @i + 1
END
Enter fullscreen mode Exit fullscreen mode

It prints 1, 2, 3. If the DECLARE reset @x each pass, you'd get 1, 1, 1.

Docs: DECLARE @local_variable (Transact-SQL).

SELECT-assignment does nothing when nothing matches

The resolve uses SELECT @Value = .... When the item isn't in the primary table, the WHERE returns no rows. You'd assume that sets @Value to NULL. It doesn't. With SELECT-style assignment, a zero-row result leaves the variable exactly where it was.

So on a missing item the assignment runs, changes nothing, and @Value still holds whatever the previous item resolved to.

Microsoft documents this exact case in Example A on that page: a variable set to a default, a SELECT @var = ... WHERE that matches nothing, and the variable keeps the default.

Docs: SELECT @local_variable (Transact-SQL).

How the two combine

Now the failure is just mechanics:

  1. @Value is never cleared between iterations.
  2. For an item missing from the primary table, the resolve matches nothing and doesn't touch @Value.
  3. The guard IF (@Value IS NULL) OR LTRIM(RTRIM(@Value)) = '' sees a leftover non-null value, decides resolution already worked, and skips the fallback.
  4. The item gets written with the previous item's value.

The fallback that should have rescued the missing item never fires, because the stale value makes it look like there was nothing to rescue.

Fixing it

Two changes close the gap.

Clear the variable at the top of every iteration so a missing row can't carry the last value forward:

WHILE (@cnt > 0)
BEGIN
    SET @Value = NULL
    ...
END
Enter fullscreen mode Exit fullscreen mode

And for a single-value lookup like this, prefer SET with a scalar subquery over SELECT-assignment. A scalar subquery that finds no rows gives you NULL, which is what you want here, because then the IS NULL guard does its job:

SET @Value = (
    SELECT isnull(p.ITEM_VAL, f.ITEM_VAL)
    FROM @Primary p
    LEFT JOIN @Fallback f ON p.ITEM_CODE = f.ITEM_CODE
    WHERE p.ITEM_CODE = @Item_Code
      AND p.ITEM_SEQ = (SELECT MAX(pmax.ITEM_SEQ) FROM @Primary pmax WHERE pmax.ITEM_CODE = @Item_Code)
)
Enter fullscreen mode Exit fullscreen mode

It reads fine. Each behavior is harmless on its own, which is why the pair survives review. If a loop variable can outlive its iteration, assume it does.

Top comments (0)