`SET` statements for local variables are unreliable

Open
#19 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
45/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
sql
Domain
databases

Research direction

No file or test is named in the issue. Reproduce the three SQL examples to observe the inconsistent SET behavior, then trace where SET statements are parsed and where parser warnings are emitted. Done means parsing a SET statement produces a warning log without banning SET in control flow.

Written by the indexing model from the issue text.

Description

bug

With the current design of the library, SET statements have a rather weird behavior. In short, before every top-level statement all local variables are redeclared with their initial value. This is a behavior hidden from the user and is potentially dangerous.

Examples

To illustrate the current behavior, here are some examples:

  • Top-level SET

    DECLARE @A INT = 10
SET @A = 5
PRINT @A

    Will print 10. The SET is actually executed but has no effect since the variable is lost after batch execution and then redeclared with its initial value before the PRINT.

  • SET in control flow

    DECLARE @A INT = 10
IF 1 = 1
  BEGIN
    SET @A = 5
    PRINT @A
  END

    Will print 5 since the whole IF is executed as a single statement.

  • And a mixed example that shows the inconsistent behavior

    DECLARE @A INT = 10
IF 1 = 1
  BEGIN
    SET @A = 5
    PRINT @A
  END
PRINT @A

    Will produce 5 followed by a 10, combining the behavior of the previous two examples.

Solution

It's unfortunately not easy to fix this in general, but it would be nice to at least add a warning log whenever SET statements are parsed in a script.

Note that simply banning SET statements at top level is not sufficient, because of cases like the third example - hence it is not trivial to statically determine which scripts would work as expected.

Dominant language
C++
Stars
15
Forks
2
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Quantco/pytsql

All issues in Quantco/pytsql

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.