Skip to content

core: rewrite sqlc.arg() to native bind parameters in a preprocessing pass above the engines - #4537

Merged
kyleconroy merged 4 commits into
mainfrom
claude/sqlc-issue-4536-jgpsh8
Jul 31, 2026
Merged

core: rewrite sqlc.arg() to native bind parameters in a preprocessing pass above the engines#4537
kyleconroy merged 4 commits into
mainfrom
claude/sqlc-issue-4536-jgpsh8

Conversation

@kyleconroy

@kyleconroy kyleconroy commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Fixes #4536.

sqlc.arg(), sqlc.narg(), sqlc.slice(), sqlc.embed() and @name are sqlc syntax, not SQL, but they used to survive all the way through every engine's parser. The pipeline only worked because each converter happened to round-trip them as an ast.FuncCall with Func.Schema == "sqlc", and that knowledge was spread across named.IsParamFunc, three validators, two AST rewriters and the PostgreSQL engine.

The preprocessor

New package internal/sql/preprocess rewrites the query text to native SQL before any engine parser runs. A dialect-parameterized lexer skips comments, string literals, quoted identifiers and dollar-quoted strings, then replaces each construct with the engine's placeholder and records what it replaced.

syntax becomes
sqlc.arg(name) / sqlc.narg(name) the dialect's native placeholder
sqlc.slice(name) the placeholder wrapped in /*SLICE:name*/
sqlc.embed(table) table.*
@name the native placeholder, where @ is sqlc syntax
engine placeholder @name
postgresql $1 sqlc syntax
mysql ? user variable, left alone
sqlite ?1 sqlc syntax

GoogleSQL and ClickHouse are deliberately not preprocessed. They are getting their own parameter system, so they should not inherit sqlc syntax just because the preprocessor is dialect-driven. An engine with no entry in the dialect table is passed through untouched — File returns the source unchanged with an empty side table — so sqlc.arg() reaches their parsers as the function call it looks like and fails there. Absence from the table is the switch, and dialect.go says so.

The side table the preprocessor emits alongside the rewritten text holds the parameter set, embed spans, slice spans, placeholder numbers, the placeholder-style validation, and an offset map back to the original source.

Compiler

The compiler consumes that side table instead of walking the AST. Three parts needed care:

  • Numbering is authoritative from the preprocessor. Engines number bind parameters in AST-conversion order, which isn't source order — MySQL's convertSelectStmt converts ORDER BY before WHERE, so order_by_binds came out with its params swapped until the compiler started applying the preprocessor's numbers by placeholder location.
  • Embeds are matched by location rather than node identity, which distinguishes a rewritten t.* from one the user wrote.
  • Placeholder-style validation moved up too. Keeping validate.ParamRef post-rewrite broke two cases, because it counted sqlc's own placeholders as user-written. The mixed-style and numbering-gap checks now run in the preprocessor over what the user actually typed.

Deleted

internal/sql/rewrite, named.IsParamFunc/IsParamSign, validate.SqlcFunctions, validate.In, validate.ParamRef, and the sqlc.arg awareness in internal/engine/postgresql/utils.go. A missed astutils container node can no longer silently drop a parameter or an embed.

Engine changes

  • GoogleSQL converts @name to a ParamRef. That is native GoogleSQL syntax rather than sqlc syntax, and the AST rewriter that used to turn it into one is gone, so the converter has to. Parameter names now come from the column the parameter is compared against, which is what every GoogleSQL golden already expected.
  • ClickHouse tags binary and unary operators as A_Expr_Kind_OP and seeds comparison and arithmetic operators. Those are what let the analysis core type a WHERE clause at all — without them even WHERE id = ? fails. analyze_params/clickhouse covers them with native ? placeholders. This is independent of the preprocessor; happy to split it out if you'd rather this PR not touch the ClickHouse engine.

Tests

internal/sql/preprocess/testdata/<engine>/<case>/ holds input.sql, output.sql, side_table.json and — when the input is invalid — stderr.txt. TestRewrite runs one subtest per directory: 69 cases (42 postgresql, 17 mysql, 10 sqlite); -update regenerates the goldens.

side_table.json is rendered by reading the result back through the same API the compiler uses, so every case covers parameter names and nullability, embed and slice spans, numbering and the offset map:

{"number": 1, "location": 47, "origin": 34, "name": "ids", "named": true, "slice": true}

location is the ? after the /*SLICE:ids*/ marker, where the engine reports the node; origin is the sqlc.slice( it replaced.

The cases were chosen by auditing every .sql file in internal/endtoend and examples/, grouping each sqlc.* call by engine, function and argument shape, and diffing that inventory against the goldens. Every shape the corpus uses now has a test for its engine. The ones that exercise a distinct path:

  • MySQL reads "x" as a string, so a double-quoted argument keeps its case, unlike a bare reference which is folded (coalesce_params/mysql, overrides_go_types/mysql)
  • $1 is not a placeholder in MySQL and is left alone (create_view/mysql)
  • -- @sqlc-vet-disable is a vet directive in a PostgreSQL query file, where @name is sqlc syntax; it survives only because the lexer never looks inside a comment (vet_disable)
  • PostgreSQL's @> operator next to a string holding a $-path, with a cast on the argument (table_function/postgresql)

go test --tags=examples ./... passes with no failures.

Behaviour changes worth reviewing

  • sqlc_arg_invalid goldens changed. Errors now name the offending text (got "$1") instead of an AST type (got *ast.ParamRef), and point at the call rather than the statement start.
  • A bare identifier in sqlc.arg(FooBar) is lowercased for PostgreSQL and MySQL, matching what those parsers did, and preserved for SQLite. It's a FoldIdentifier flag in the dialect table if that split is wrong.

🤖 Generated with Claude Code

claude added 4 commits July 31, 2026 15:39
sqlc.arg(), sqlc.narg(), sqlc.slice(), sqlc.embed() and @name are sqlc
syntax, not SQL, but they used to survive all the way through every
engine's parser. The pipeline only worked because each converter happened
to round-trip them as an ast.FuncCall with Func.Schema == "sqlc", and that
knowledge was scattered across named.IsParamFunc, three validators, two
AST rewriters and the PostgreSQL engine.

Add internal/sql/preprocess, which rewrites the query text to native SQL
before any engine parser runs. A dialect-parameterized lexer skips
comments, string literals, quoted identifiers and dollar-quoted strings,
then replaces each construct with the engine's placeholder ($1, ?, ?1 or
@name) and records what it replaced: the parameter set, embed spans, slice
spans, placeholder numbers and an offset map back to the original text.

The compiler consumes that side table instead of walking the AST:

  - parameter numbering comes from the preprocessor in source order, and
    is applied to the parsed AST by placeholder location, so an engine
    that converts nodes out of order no longer reorders a query's params
  - embeds are matched by location rather than node identity, which tells
    a rewritten "t.*" apart from one the user wrote
  - placeholder-style validation (mixed styles, numbering gaps) and
    sqlc.* validation move into the preprocessor, where they can report
    the span of the actual call

Deleted: internal/sql/rewrite, named.IsParamFunc/IsParamSign,
validate.SqlcFunctions, validate.In, validate.ParamRef, and the
sqlc.arg awareness in the PostgreSQL engine. A missed astutils container
node can no longer silently drop a parameter or an embed.

Engine changes are limited to what the removal exposed: GoogleSQL now
converts @name to a ParamRef like every other engine, and ClickHouse
tags binary/unary operators as A_Expr_Kind_OP and seeds comparison and
arithmetic operators so the analysis core can type a WHERE clause. With
those in place sqlc.arg/narg work for ClickHouse with no sqlc-specific
engine code at all.

Error messages for invalid sqlc.* calls now name the offending text
rather than an AST type, and point at the call instead of the statement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BSM9xGoosq58r1Mz6B7YP
The rewritten SQL only shows half of what the preprocessor produces. The
other half — parameter names and nullability, embed and slice spans,
placeholder numbering and the offset map back to the original text — was
covered by a handful of hand-written Go tests that each poked at one
case.

Record it as a golden instead. Every testdata directory now holds a
side_table.json alongside its output.sql, rendered by reading the result
back through the same API the compiler uses, so all 59 cases exercise the
whole side table rather than the six that had a bespoke test. The
hand-written tests are gone.

Two supporting changes:

  - Result.Statements() exposes the statements in source order, so the
    side table can be walked without looking each one up by offset
  - the whitespace trailing the final semicolon is no longer reported as
    a statement; it is still written to the output, it is just not
    something a query can be found in

Renaming the errors helper to cover ParamErr as well as Err turned up two
cases whose input also tripped the numbering-gap check. Their inputs are
now valid so each case tests one thing, and the gap and mixed-style
errors get cases of their own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BSM9xGoosq58r1Mz6B7YP
Audited every .sql file in internal/endtoend (and examples/), grouping
each sqlc.* call by engine, function and argument shape, then diffed that
inventory against the preprocessor's testdata. Seventeen shapes the corpus
relies on had no golden. They all behaved correctly; they just were not
pinned.

The ones that exercise a distinct code path:

  - MySQL reads "x" as a string, so a double-quoted argument names the
    parameter and keeps its case, unlike a bare reference which is folded
    (coalesce_params/mysql, overrides_go_types/mysql)
  - $1 is not a placeholder in MySQL and is left alone
    (create_view/mysql)
  - "-- @sqlc-vet-disable" is a vet directive in a PostgreSQL query file,
    where @name *is* sqlc syntax; it survives only because the lexer never
    looks inside a comment (vet_disable)
  - PostgreSQL's @> operator next to a string holding a $-path, with a
    cast on the argument (table_function/postgresql)

The rest fill in per-engine shapes: string-constant arguments for narg and
slice on PostgreSQL and SQLite, narg against a bare reference on MySQL and
SQLite, and the four invalid-call errors on MySQL and SQLite, where the
reported text is dialect-specific ('got "?"' rather than 'got "$1"').

Also adds cases for the string-constant forms that had only one golden
between them: case preservation against a folded bare reference, an
escaped quote inside the name, and sqlc.embed('t').

76 cases in total. Every sqlc syntax shape in the corpus now has a
preprocessor test for its engine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BSM9xGoosq58r1Mz6B7YP
Both engines are getting their own parameter system, so they should not
pick up sqlc.arg()/narg()/slice()/embed() by default just because the
preprocessor happens to be dialect-driven.

Drop their entries from the dialect table. An engine with no entry is now
passed through untouched: File returns the source unchanged with an empty
side table, so sqlc.arg() reaches the parser as the function call it looks
like and fails there. Leaving an engine out of the table is the switch,
and dialect.go says so.

GoogleSQL still converts "@name" to a ParamRef. That is native GoogleSQL
syntax rather than sqlc syntax, and the AST rewriter that used to turn it
into one is gone, so the converter has to. Parameter names now come from
the column the parameter is compared against, which is what every
GoogleSQL golden already expected.

The ClickHouse operator work stays, retargeted off sqlc syntax:
A_Expr_Kind_OP on binary and unary operators plus the seeded comparison
and arithmetic operators are what let the analysis core type a WHERE
clause at all, and analyze_params/clickhouse now covers them with native
"?" placeholders.

Preprocessor testdata drops to 69 cases across the three engines that are
actually preprocessed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BSM9xGoosq58r1Mz6B7YP
@kyleconroy
kyleconroy merged commit 5ce31fc into main Jul 31, 2026
13 checks passed
@kyleconroy
kyleconroy deleted the claude/sqlc-issue-4536-jgpsh8 branch July 31, 2026 17:40
kyleconroy pushed a commit that referenced this pull request Jul 31, 2026
#4537 rewrites sqlc's syntax to native placeholders above the engines, and
SQLite is one of the preprocessed dialects, so sqlc.arg() and @name never
reach the parser. The fold this branch carried is dead.

One gap follows from the pass's rule that a statement whose sqlc syntax
did not validate is copied through for the engine to parse: that assumes
the engine can parse it. SQLite cannot, so an invalid sqlc.arg() surfaced
as `near "(": syntax error` and the preprocessor's own diagnostics were
never reported -- from parseCatalog, since a schema file and a query file
are often the same file. On a parse failure the compiler now reports the
sqlc syntax errors recorded for that file, if any, in place of the failure
they produced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5
kyleconroy added a commit that referenced this pull request Aug 1, 2026
* sqlite: replace the ANTLR parser with meyer

meyer is a hand-written recursive descent parser for SQLite's dialect,
ported from src/parse.y and src/tokenize.c and checked against SQLite
itself. Moving to it deletes 1.2 MB of generated ANTLR code and the
antlr4-go dependency, and replaces a grammar that only approximated
SQLite with one that accepts what SQLite accepts.

parse.go calls meyer and slices statement extents out of the node spans;
convert.go maps meyer's tree onto sqlc's AST; reserved.go now defers to
meyer's keyword table, which the hand-maintained list was missing
MATERIALIZED and RETURNING from.

SQLite has no schema-qualified function call, so sqlc.arg() and its
siblings are not SQL meyer will parse. The parser folds "sqlc.name(" to
"sqlc_name(" over the token stream -- one byte for another, so every
offset still points into the original source, and string literals and
comments are untouched -- and the converter puts the schema back.

Behavior the old grammar got wrong and this corrects:

- "col TEXT NULL" was read as a column of type "TEXTNULL", so the column
  generated as interface{} rather than a nullable string. This changes
  one golden file.
- GROUP BY was dropped whenever the query also had a WHERE clause, from
  an off-by-one over the ANTLR expression list. Now that GROUP BY reaches
  the compiler, its column references are validated, which SQLite's
  implicit rowid would have failed; the compiler now recognizes rowid,
  _rowid_ and oid for SQLite.
- Comparison operators other than "=" were all recorded as "=", and IS,
  ISNULL, NOTNULL and LIKE reached the AST as "=" too. They now carry
  their own spelling, and lang.IsComparisonOperator learned the
  keyword-spelled comparisons so they still type as bool.
- ORDER BY, DISTINCT, WITH RECURSIVE, ON CONFLICT, OVER and FILTER were
  parsed and then discarded, so ast.Format dropped them from the SQL it
  rebuilds. All are carried through now.
- Quoted identifiers other than "..." kept their quotes; float literals
  became the integer 0.

One capability is lost: the pinned SQLite build meyer targets defines
neither SQLITE_ENABLE_UPDATE_DELETE_LIMIT nor SQLITE_UDL_CAPABLE_PARSER,
so UPDATE and DELETE no longer accept ORDER BY or LIMIT. No test covered
it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5

* sqlite: drop parse_test.go

The end-to-end tests are the acceptance gate for the parser.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5

* sqlite: drop catalog_test.go

The end-to-end tests cover the catalog the schema builds. Removing the
test leaves newTestCatalog with no callers, so it goes too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5

* sqlite: drop the analyzer test

The end-to-end tests exercise the analyzer against a real database.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5

* sqlite: drop the sqlc.arg fold, now that preprocessing handles it

#4537 rewrites sqlc's syntax to native placeholders above the engines, and
SQLite is one of the preprocessed dialects, so sqlc.arg() and @name never
reach the parser. The fold this branch carried is dead.

One gap follows from the pass's rule that a statement whose sqlc syntax
did not validate is copied through for the engine to parse: that assumes
the engine can parse it. SQLite cannot, so an invalid sqlc.arg() surfaced
as `near "(": syntax error` and the preprocessor's own diagnostics were
never reported -- from parseCatalog, since a schema file and a query file
are often the same file. On a parse failure the compiler now reports the
sqlc syntax errors recorded for that file, if any, in place of the failure
they produced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5

* sqlite: update to meyer v0.1.1, restoring UPDATE/DELETE LIMIT

v0.1.1 adds parser.Options, whose UpdateDeleteLimit field compiles in the
grammar SQLITE_ENABLE_UPDATE_DELETE_LIMIT does. The option selects between
two forms of SQLite's own rules, so meyer cannot tell which one a caller
wants from the SQL alone; sqlc has always accepted ORDER BY and LIMIT on
UPDATE and DELETE, so it asks for them.

UpdateStmt and DeleteStmt grow OrderBy and Limit fields to match. sqlc's
AST has somewhere to put the LIMIT and nowhere to put the ORDER BY, which
is what the ANTLR converter did with them too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkrYmzwTktB2CA5Bvqg6z5

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite sqlc.arg() to native bind parameters in a preprocessing pass above the engines

2 participants