Describe the bug
pgls_tokenizer already recognises every named-parameter style, with explicit doc
comments pointing at psql interpolation and sqlc:
/// e.g. `:name` (raw substitution) -> NamedParamKind::ColonRaw
/// e.g. `:'name'` (quoted string) -> NamedParamKind::ColonString
/// e.g. `:"name"` (quoted identifier) -> NamedParamKind::ColonIdentifier
/// e.g. `@name` (sqlc) -> NamedParamKind::AtPrefix
/// e.g. `$name` -> NamedParamKind::DollarRaw
But NamedParam has no consumer outside the tokenizer:
$ grep -rn "NamedParam" --include="*.rs" crates/ | grep -v pgls_tokenizer | grep -v test
(no output)
The raw text still reaches libpg_query, which rejects it, and the whole statement is
reported as a syntax error. So the tokenizer declares support for a feature that the
pipeline then discards.
To Reproduce
repro.sql:
SELECT
customers.id
FROM staging.customers
CROSS JOIN :raw_data.migration_infos;
$ postgres-language-server check repro.sql
repro.sql:1:1 syntax ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Invalid statement: syntax error at or near "$1"
Nothing else in the file is analysed: no completion, no lint, no typecheck.
Impact
Measured on a real 1436-file SQL codebase where :schema placeholders are used to
parameterise the target schema at execution time:
|
Files with syntax errors |
Syntax diagnostics |
| Today |
466 |
1768 |
| With named parameters substituted before parsing |
224 |
336 |
242 files and 1432 diagnostics, 81% of them, come from this single gap. The remaining
ones are unrelated (blank line inside a statement, filed separately).
Expected behavior
Named parameters are substituted with syntactically valid placeholder text before the
statement is handed to libpg_query, and diagnostic positions are mapped back to the
original text.
Implementation
I would like to open a PR for this. Sketch, for review before I write it:
-
Where. In pgls_workspace, where the document content is split and handed to the
parser (workspace/server/document.rs), run a substitution pass over the tokenizer
output before parsing.
-
What to substitute with. Replacing :name with the bare identifier name is
syntactically valid in both positions it can occur:
- qualifier position:
:raw_data.t becomes raw_data.t
- value position:
WHERE x = :id becomes WHERE x = id
This is the minimal change that makes the statement parse. Type checking may then
report an unresolved column for a value-position parameter, which is still strictly
better than discarding the whole file. A configurable mapping could come later; it is
not needed to close this gap.
-
Mapping positions back. TextRangeReplacementBuilder in pgls_text_size already
does exactly this, including for replacements of a different length, via
to_original_position / to_original_range. No new machinery required.
One caveat worth knowing
Array slice syntax collides with ColonRaw. In
SELECT array_to_string(arr[3:array_upper(arr, 1)], ',');
the :array_upper is a slice bound, not a parameter. The ':' branch of the tokenizer
is context-free and emits OpenBracket without tracking depth, so it will classify this
as a NamedParam. Substituting it produces invalid SQL.
I hit exactly this while measuring the numbers above: it was the single remaining
failure out of 1436 files. The substitution pass therefore needs bracket-depth awareness
at the consumer level, since the tokenizer's cursor is flat.
Tests I would add
- each of the five
NamedParamKind variants parses after substitution
- a diagnostic on a statement containing a named parameter reports the original,
pre-substitution position
arr[3:array_upper(arr, 1)] is left untouched
Happy to adjust any of this before writing it, in particular the choice of replacement
text, which is the one decision with user-visible consequences.
System information
- postgres-language-server 0.25.7
- macOS (aarch64), release binary
postgres-language-server_aarch64-apple-darwin
Describe the bug
pgls_tokenizeralready recognises every named-parameter style, with explicit doccomments pointing at psql interpolation and sqlc:
But
NamedParamhas no consumer outside the tokenizer:The raw text still reaches
libpg_query, which rejects it, and the whole statement isreported as a syntax error. So the tokenizer declares support for a feature that the
pipeline then discards.
To Reproduce
repro.sql:Nothing else in the file is analysed: no completion, no lint, no typecheck.
Impact
Measured on a real 1436-file SQL codebase where
:schemaplaceholders are used toparameterise the target schema at execution time:
242 files and 1432 diagnostics, 81% of them, come from this single gap. The remaining
ones are unrelated (blank line inside a statement, filed separately).
Expected behavior
Named parameters are substituted with syntactically valid placeholder text before the
statement is handed to
libpg_query, and diagnostic positions are mapped back to theoriginal text.
Implementation
I would like to open a PR for this. Sketch, for review before I write it:
Where. In
pgls_workspace, where the document content is split and handed to theparser (
workspace/server/document.rs), run a substitution pass over the tokenizeroutput before parsing.
What to substitute with. Replacing
:namewith the bare identifiernameissyntactically valid in both positions it can occur:
:raw_data.tbecomesraw_data.tWHERE x = :idbecomesWHERE x = idThis is the minimal change that makes the statement parse. Type checking may then
report an unresolved column for a value-position parameter, which is still strictly
better than discarding the whole file. A configurable mapping could come later; it is
not needed to close this gap.
Mapping positions back.
TextRangeReplacementBuilderinpgls_text_sizealreadydoes exactly this, including for replacements of a different length, via
to_original_position/to_original_range. No new machinery required.One caveat worth knowing
Array slice syntax collides with
ColonRaw. Inthe
:array_upperis a slice bound, not a parameter. The':'branch of the tokenizeris context-free and emits
OpenBracketwithout tracking depth, so it will classify thisas a
NamedParam. Substituting it produces invalid SQL.I hit exactly this while measuring the numbers above: it was the single remaining
failure out of 1436 files. The substitution pass therefore needs bracket-depth awareness
at the consumer level, since the tokenizer's cursor is flat.
Tests I would add
NamedParamKindvariants parses after substitutionpre-substitution position
arr[3:array_upper(arr, 1)]is left untouchedHappy to adjust any of this before writing it, in particular the choice of replacement
text, which is the one decision with user-visible consequences.
System information
postgres-language-server_aarch64-apple-darwin