Skip to content

Named parameters are tokenized but never substituted, so any file using them fails to parse #787

Description

@edjubert

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:

  1. 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.

  2. 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.

  3. 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

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

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions