Fold plpgsql identifiers the way Postgres does

[reltuk]

Declarations were already folded but references used the source text as written. DECLARE MyVar was unreachable as MYVAR, quoted declarations and references were poorly handled, and a trigger body could not write new/old in lowercase because TriggerCall had registered the identifiers as NEW/OLD.

Fold variable names at reference sites in the source text, including in expression references, assignment targets, RAISE arguments and EXECUTE USING arguments. Correspondingly register lowercase names for NEW/OLD and TG_ variables.

We needed to add variable name quoting in the two places where the engine generates SQL that mentions a variable whose declared name we already hold: the integer FOR loop's condition and increment, and the CASE temporary.

[github-actions]

	Main	PR
Total	42090	42090
Successful	19246	19251
Failures	22844	22839
Partial Successes1	5469	5463

	Main	PR
Successful	45.7258%	45.7377%
Failures	54.2742%	54.2623%

${\color{lightgreen}Progressions (2)}$

plpgsql

QUERY: update PField set name = 'PF0_2' where name = 'PF0_X';

triggers

QUERY: insert into parted values (0, 1, 'zero win');

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[itoqa]

Commit: 3743010: 21 test cases ran, 19 passed ✅, 2 additional findings ⚠️.

Summary

Coverage spans database function behavior across normal flows, identifier casing, variable scope, record and trigger handling, conditional branches, dynamic SQL, error reporting, transaction rollback, and loop boundary cases. It also includes adversarial checks for invalid trigger actions, parameter mismatches, failed operations, and concurrent session isolation, with the exercised behavior broadly healthy aside from unrelated pre-existing defects.

Safe to merge — the observed failures are medium-severity, unrelated database-engine defects and are explicitly not attributable to this PR, with no regressions or newly introduced failures identified. They are appropriate for follow-up rather than merge blockers.

Tests run by Ito

Result	Severity	Type	Description
✅	—	Assignment	The function used the same value for assignment, SELECT INTO, RAISE, and dynamic SQL. It returned a:a:a and the notice showed the bound value a.
✅	—	Assignment	RAISE reports a clear error when a message has too few or too many values. The identifier changes did not hide or change these existing checks.
✅	—	General	Quoted loop variables keep working when the start and end values are equal, when the range is empty, and when the loop counts backward. The focused regression completed successfully after the local test container was rebuilt.
✅	—	General	Verified acceptable by independent adversarial review: the reported expectation does not match what the code actually promises. Review notes: The finding incorrectly treats the absence of a savepoint inside the PL/pgSQL operation loop as proof that prior writes survive. The function's nested statements execute as part of the caller's statement transaction, and the connection error path rolls that transaction back; the repository also codifies this behavior for a write followed by division by zero. A readback showing retained data theref…
✅	—	General	An invalid trigger action rejected the insert and removed both the new row and the audit entry. A later valid insert worked normally.
✅	—	General	Insert, update, and delete triggers returned the correct row values, and the table was empty after the full sequence.
✅	—	General	The first matching branch ran, later matches were selected when needed, and the fallback branch ran for other inputs. Each result kept its added suffix after the CASE finished.
✅	—	General	The concurrent check could not reach the local SQL service because the available sessions lacked credentials and the service could not be restarted. Source review shows each function call starts with its own binding state, so no cross-call defect was found.
✅	—	Binding	PL/pgSQL variables worked correctly when written with different casing, while quoted names stayed distinct. Assignments, SELECT INTO, expressions, and trigger record access all behaved as expected.
✅	—	Binding	Assigned record fields returned the expected values, and reading an unassigned record returned the expected error.
✅	—	Binding	A function call still ran when a local variable had the same name. The call returned the expected value of 3.
✅	—	Case	The function received 7 and returned the result from the matching later WHEN branch: later.
✅	—	Case	A CASE expression with no matching WHEN condition returned its fallback value, so the ELSE path works as expected.
✅	—	Loop	The quoted loop variable kept its mixed-case name while the loop ran from 1 to 3. The function returned the expected sum of 6.
✅	—	Loop	The reverse loop visits the expected values and returns the correct total. The earlier attempt could not run because the local test setup was unavailable, but the focused regression passed after a Go-capable test container was provisioned.
✅	—	Trigger	The trigger inserted the valid row and rejected the negative row with the expected error.
✅	—	Trigger	Using an unsupported trigger name rejected the insert, and the table stayed empty.
✅	—	Variable	The function returned 1 when the declared variable and its later reference used different letter casing. A quoted mixed-case variable also returned 1 when referenced with its exact spelling.
✅	—	Variable	A function can create a variable whose quoted name contains a quote, assign text to it, and return that text through the matching name.
⚠️		General	The function returned the inner value after the nested block ended. The first reproduction expected 3 but got 7, because the call received 6; the retry added a record value and expected 13 but got 17, confirming the same leaked value while the record value stayed correct.
⚠️		Rev	The insert returned ERROR: column "tg_op" could not be found in any table in scope (SQLSTATE 42703). The expected trigger message containing op=INSERT name=rev4_trg was not produced, and the follow-up query showed that the table was still empty.

Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Nested variable value leaks after block ends

• Severity: Medium
• Description: The function returned the inner value after the nested block ended. The first reproduction expected 3 but got 7, because the call received 6; the retry added a record value and expected 13 but got 17, confirming the same leaked value while the record value stayed correct.
• Impact: Database functions that reuse a variable name inside a nested block can return the wrong value after the block ends, causing incorrect calculations. Users can avoid the issue by using different variable names, but affected functions remain wrong until changed.
• Steps to Reproduce:
  1. Create a SQL function that declares outerv as 2.
  2. Inside the function, declare another outerv as 5 and increment the inner value to 6.
  3. End the inner block and return a function call using outerv; the expected outer-scope value is 2, so the function should return 3.
  4. Run the function through the local Go SQL test harness and compare the returned value with 3.
• Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
• Code Analysis: The production path explicitly models lexical block scopes. Block.AppendOperations in server/plpgsql/statements.go:97-155 pushes a compile-time scope, emits OpCode_ScopeBegin at lines 110-114, registers declarations at lines 115-144, emits the nested body, then emits OpCode_ScopeEnd and pops the compile-time scope at lines 151-154. At runtime, interpreter_logic.go:393-396 handles those opcodes with stack.PushScope and stack.PopScope. Variable substitution in statements.go:499-545 records the normalized binding name in the operation's SecondaryData, and InterpreterStack.GetVariableWithError in interpreter_stack.go:197-252 searches the runtime frames from the newest frame outward. In the failing case, the post-block reference is still resolved to the inner normalized outerv binding instead of the restored outer frame. The mismatch is therefore in the interaction between compiled binding names and runtime scope restoration, not in the record arithmetic or the SQL function call. The smallest practical fix is to preserve a scope-specific binding/reference for each compiled operation, or otherwise ensure that OpCode_ScopeEnd removes the inner binding and subsequent lookups restart at the parent frame; add the nested-shadowing regression test before changing broader identifier behavior.

Evidence Package

• text/BF-ORDER-1/BF-ORDER-1-nested-shadowing.txt — 2 KB

🟡 Insert triggers cannot read operation metadata

• Severity: Medium
• Description: The insert returned ERROR: column "tg_op" could not be found in any table in scope (SQLSTATE 42703). The expected trigger message containing op=INSERT name=rev4_trg was not produced, and the follow-up query showed that the table was still empty.
• Impact: Database inserts that use a trigger to read its operation metadata are rejected, so no row is saved. This blocks the affected trigger-backed workflow until the trigger is changed or the defect is fixed.
• Steps to Reproduce:
  1. Create a table with an integer column.
  2. Create a PL/pgSQL trigger function that raises an exception using lowercase tg_op and tg_name.
  3. Create a BEFORE INSERT trigger that calls the function.
  4. Insert one row and inspect the error and the table contents.
• Stub / mock content: The test used the local in-process Go RunScripts harness with authentication bypassed and an isolated SQL session. No stubs, mocks, route interceptions, or test-only code changes were applied to the application.
• Code Analysis: server/node/trigger_execution.go:103-113 derives the trigger operation by type-switching on te.Source. It sets tgOp only for an exact *plan.InsertInto, *plan.Update, *plan.DeleteFrom, or *plan.Truncate. The analyzer's server/analyzer/assign_triggers.go:195-207 intentionally sets the trigger source for an INSERT to node.Source, and server/analyzer/assign_triggers.go:232-234 wraps that source inside the original InsertInto node. Therefore the source observed by TriggerExecution can be a wrapped child rather than the exact top-level InsertInto, leaving tgOp as the empty string. In server/node/trigger_execution.go:217-221, the executor adds TG_OP to triggerVars only when tgOp is non-empty, so this path passes no operation value to the PL/pgSQL interpreter. The PR's server/plpgsql/interpreter_logic.go:83-90 normalizes supplied trigger variable names and server/plpgsql/interpreter_logic.go:528-541 registers the folded name tg_op, but normalization cannot bind a variable that the executor never supplies. The smallest practical fix is to derive the operation from stable trigger-execution context or pass it explicitly when the analyzer constructs the wrapper, rather than relying on the runtime source node's exact concrete type; the fix belongs in the unchanged trigger-execution/analyzer handoff.

Evidence Package

• text/REV-4/REV-4-trigger-metadata-failure.txt — 7 KB

Tip

Reply with @itoqa to send us feedback on this test run.

[coffeegoddd]

@reltuk DOLT

read_tests	from_latency	to_latency	percent_change
covering_index_scan_postgres	2.43	2.43	0.0
groupby_scan_postgres	74.46	75.82	1.83
index_join_postgres	2.26	2.18	-3.54
index_join_scan_postgres	1.61	1.55	-3.73
index_scan_postgres	493.24	493.24	0.0
oltp_point_select	0.37	0.36	-2.7
oltp_read_only	6.43	6.32	-1.71
select_random_points	0.7	0.7	0.0
select_random_ranges	1.01	1.03	1.98
table_scan_postgres	493.24	493.24	0.0
types_table_scan_postgres	1235.62	1235.62	0.0

write_tests	from_latency	to_latency	percent_change
oltp_delete_insert_postgres	6.67	6.67	0.0
oltp_insert	3.36	3.3	-1.79
oltp_read_write	13.22	13.22	0.0
oltp_update_index	3.55	3.55	0.0
oltp_update_non_index	3.25	3.25	0.0
oltp_write_only	6.91	7.04	1.88
types_delete_insert_postgres	7.17	7.17	0.0