Skip to content

Add PostgreSQL $N and SQLite :name placeholder syntaxes (#263)#264

Open
aleks-f wants to merge 6 commits into
hyrise:mainfrom
aleks-f:263-placeholders
Open

Add PostgreSQL $N and SQLite :name placeholder syntaxes (#263)#264
aleks-f wants to merge 6 commits into
hyrise:mainfrom
aleks-f:263-placeholders

Conversation

@aleks-f
Copy link
Copy Markdown

@aleks-f aleks-f commented May 27, 2026

Extends the parser to recognise two placeholder forms alongside the existing ?:

  • $N (PostgreSQL 1-based explicit numbering, e.g. WHERE a = $1)
  • :name (SQLite named, e.g. WHERE a = :user)

The ? path is unchanged at every layer (lexer, parser, AST, result ordering); existing tests under test/prepare_tests.cpp pass untouched.

Implementation:

  • New lexer rules for $[0-9]+ and :[A-Za-z][A-Za-z0-9_]* placed before the punctuation catch-all. $ was previously unused; : was in the punctuation class but never referenced by any grammar rule, so the longer :identifier match wins without conflict.
  • New bison tokens DOLLAR_PARAM (carries the int N) and NAMED_PARAM (carries the identifier). param_expr now has three alternatives. $0 is rejected via YYERROR at parse time.
  • Three ExprType values appended (no ordinal change): kExprParameter for ?, kExprParameterDollar for $N (ival = N, 1-based), and kExprParameterNamed for :name (name = identifier). Distinct enum values rather than one with discriminator fields keep consumer switch statements explicit and round-trip printing straightforward.
  • Factory functions Expr::makeDollarParameter and makeNamedParameter. isLiteral returns true for all three. The destructor already frees name so no further changes are needed.
  • The top-level input rule's renumber loop only touches kExprParameter; $N keeps its explicit ival, :name keeps its name. SQLParserResult::addParameter still sorts by ival, which now orders $N by explicit position and clusters named placeholders.
  • sqlhelper print case extended to handle the two new ExprType values.

Tests:

  • Three new cases in test/prepare_tests.cpp covering $N, $N declared out of order (verifying ival-sorted result vector), and :name.
  • Three new good queries and two new bad queries ($0, lone $) in test/queries/.
  • bison -v reports no new shift/reduce or reduce/reduce conflicts.
  • make test passes all three checks (SQL tests, valgrind, grammar).

Extends the parser to recognise two placeholder forms alongside the
existing ?:

- $N (PostgreSQL 1-based explicit numbering, e.g. WHERE a = $1)
- :name (SQLite named, e.g. WHERE a = :user)

The ? path is unchanged at every layer (lexer, parser, AST, result
ordering); existing tests under test/prepare_tests.cpp pass untouched.

Implementation:

- New lexer rules for \$[0-9]+ and :[A-Za-z][A-Za-z0-9_]* placed
  before the punctuation catch-all. $ was previously unused; : was
  in the punctuation class but never referenced by any grammar rule,
  so the longer :identifier match wins without conflict.
- New bison tokens DOLLAR_PARAM (carries the int N) and NAMED_PARAM
  (carries the identifier). param_expr now has three alternatives.
  $0 is rejected via YYERROR at parse time.
- Three ExprType values appended (no ordinal change): kExprParameter
  for ?, kExprParameterDollar for $N (ival = N, 1-based), and
  kExprParameterNamed for :name (name = identifier). Distinct enum
  values rather than one with discriminator fields keep consumer
  switch statements explicit and round-trip printing straightforward.
- Factory functions Expr::makeDollarParameter and makeNamedParameter.
  isLiteral returns true for all three. The destructor already frees
  name so no further changes are needed.
- The top-level input rule's renumber loop only touches
  kExprParameter; $N keeps its explicit ival, :name keeps its name.
  SQLParserResult::addParameter still sorts by ival, which now
  orders $N by explicit position and clusters named placeholders.
- sqlhelper print case extended to handle the two new ExprType
  values.

Tests:

- Three new cases in test/prepare_tests.cpp covering $N, $N
  declared out of order (verifying ival-sorted result vector), and
  :name.
- Three new good queries and two new bad queries ($0, lone $) in
  test/queries/.
- bison -v reports no new shift/reduce or reduce/reduce conflicts.
- make test passes all three checks (SQL tests, valgrind, grammar).
@aleks-f
Copy link
Copy Markdown
Author

aleks-f commented May 27, 2026

I just noticed bison upgrade also went in. If you want that reverted, let me know.

…XECUTE

Two improvements on top of the placeholder support landed in hyrise#263:

1. Placeholder Expr nodes now record their source column (0-based) in
   Expr::ival2 across all three kinds. For ? the column is
   yylloc.total_column - 1, for $N total_column - (1 + digits of N),
   for :name total_column - 1 - strlen(name). ival2 was previously
   written once for ? (yyloc.param_list.size(), which duplicated the
   sequential id later stored in ival by the top-level renumber loop)
   and not read by any consumer. Repurposing it for the source column
   gives all three placeholder kinds a consistent meaning: ival is
   the binding identity (sequential for ?, explicit N for $N, 0 for
   :name); ival2 is the source column. This lets consumers walk
   parameters() sorted by ival2 and do text-based placeholder
   substitution without re-scanning the input.

2. The extended_literal rule no longer rejects ? placeholders. The
   check at bison_parser.y:1098 was inconsistent with the newer
   $N / :name placeholders, which the same rule already accepts.
   Common patterns like INSERT INTO t VALUES(?, ?, ?) and
   EXECUTE foo(?) now parse natively. Two queries-bad.sql cases that
   asserted the rejection move to queries-good.sql.

prepare_tests.cpp gains ival2 assertions on the four placeholder
cases validating computed source columns match hand-counted offsets.

Verified: make test passes all three checks (SQL tests, valgrind,
grammar conflict).
aleks-f added 4 commits May 27, 2026 17:08
The macro had a '!= = 0' typo that would fail to compile if ever
expanded. No callers currently use ASSERT_STRNEQ so the typo was
dormant, but it is still broken on its face. Correct the condition
to '== 0' (the inverse of ASSERT_STREQ's '!= 0').
ColumnConstraints owns two heap-allocated members (constraints and
references, the latter containing heap-allocated ReferencesSpecification
pointers). On the happy path those pointers transfer into a
ColumnDefinition which deletes them in its destructor. On parse error
bison reaches the wildcard %destructor (delete $$), which only frees
the wrapper struct and leaks the inner members.

Add a dedicated %destructor for <column_constraints_t> that releases
the inner heap-allocated state before deleting the wrapper. valgrind
on queries-good.sql + queries-bad.sql now reports 14,545 allocs /
14,545 frees, no leaks.
bison_parser.cpp #includes flex_lexer.h, which is generated alongside
flex_lexer.cpp from flex_lexer.l. Under parallel make (with -j>1), the
compilation of bison_parser.o could start before flex regenerated
flex_lexer.h, producing 'hsql_lex was not declared' errors.

Add flex_lexer.cpp as an explicit prerequisite of bison_parser.o so make
serializes correctly. The reverse dep (flex_lexer.o depends on
bison_parser.cpp) was already wired up; this completes the pair.
The macros expanded to a statement followed by a trailing
static_assert(true, "...") that enforced a closing semicolon. The
expansion was technically a statement + declaration sequence, which
breaks unbraced control-flow contexts (e.g. if (c) ASSERT_TRUE(x); else
...) due to dangling-else / declaration-as-if-body rules.

Wrap each assertion macro body in do { ... } while (false). The
trailing semicolon at the call site now naturally terminates the loop;
the macro is a single statement that composes safely in any context.
Drop the static_assert(true, ...) suffix since it is no longer needed.
ASSERT_EQ / ASSERT_NEQ gain a semicolon after their inner ASSERT call,
which previously relied on the suffix to absorb the user's trailing ';'.
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.

1 participant