Skip to content

&sql(...) then SQLCODE always throws <UNDEFINED> sqlSQLCODE1 — #145 fixed the producer and left the consumer rewrite behind #177

Description

@PYDuquesnoy

Reported by the skills session against 0.13.0; reproduced at the wire on 0.15.1, namespace APP, dev instance :43080.

The single most canonical embedded-SQL idiom there is — &sql(...) then read SQLCODE — throws every time.

What happens

&sql(SELECT ID, Name FROM Ens_Config.Production WHERE Name = 'Ferry.Production')
if SQLCODE = 0 { write "Found production: " _ ID, ! }

translated_code:

[1] set sqlrs1 = ##class(%SQL.Statement).%New()
[2] set sqlsc1 = sqlrs1.%Prepare("SELECT ID, Name FROM Ens_Config.Production WHERE ...")
[3] set sqlrs1 = sqlrs1.%Execute() set SQLCODE=$Select($IsObject(sqlrs1):sqlrs1.%SQLCODE,1:-400),%ROWCOUNT=...
[4]
[5] if sqlSQLCODE1 = 0 {

<UNDEFINED> ... sqlSQLCODE1.

The producer sets SQLCODE. The consumer was rewritten to sqlSQLCODE1. Nothing anywhere sets that name.

It is broader than the report

The skills session hit the no-INTO form. The SELECT ... INTO form — the one our own test suite covers — fails too, and fails only when the query succeeds:

&sql(SELECT Name INTO :nm FROM %Dictionary.ClassDefinition WHERE Name = 'Ens.Director')
write "sqlcode=", SQLCODE, " nm=", $G(nm), !

<UNDEFINED> ... sqlSQLCODE1

Because translate_select_into emits set sqlSQLCODE1 = sqlrs1.%SQLCODE inside the else branch only. Find a row and the variable is never assigned; find nothing and it is. So the failure mode is inverted: the happy path is the broken one.

Positive control: write "alive",! and the same statement without a SQLCODE read both return normally, so this is the rewrite, not the connection or the execute path.

Root cause

sqlcode_epilogue (#145) moved the producer to the REAL names, SQLCODE and %ROWCOUNT. Its own doc comment says the next-line rewrite below it is now "a belt-and-braces path rather than the only one".

It is not belt-and-braces. It is a renamer, and it renames the read to a variable #145 stopped producing. The evidence that the producer side already moved on is sitting in the signature two functions down:

fn translate_select_no_into(sql: &str, rs_var: &str, sc_var: &str, _sqlcode_var: &str) -> String

Underscore-prefixed — that path stopped using the variable. rewrite_next_line_sqlcode is still handed it.

Same shape as the last three of these: a fix landed on one layer and the sibling layer kept the old contract, which made the sibling look more trustworthy, not less.

And our tests lock the bug in. test_sql_translate.rs:129:

assert!(!r.translated_code.contains("\nif SQLCODE"), "bare SQLCODE on next line should be rewritten");
assert!(r.translated_code.contains("sqlSQLCODE"), "should contain generated SQLCODE var");

That asserts the defect. It passed through #145 unchanged because #145 only added assertions about the producer and never asked whether the existing ones still described something we wanted.

Fix

The regression test to keep is the one that would have caught this: a SELECT INTO that FINDS a row and then reads SQLCODE runs green.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions