Actionable comments posted: 4

In `@core/src/main/java/io/questdb/griffin/SqlCompilerImpl.java`:
- Around line 392-401: The current post-compile check in SqlCompilerImpl (after
calling compileInner) only inspects one token via SqlUtil.fetchNext(lexer) and
treats a single semicolon as OK, allowing inputs like "select 1; select 2";
change the logic to consume and ignore any number of trailing semicolon tokens
(use isSemicolon(tok) in a loop) and only treat a non-null, non-semicolon token
as an unexpected-token error (throw SqlException using
lexer.lastTokenPosition()), while still skipping the entire check when
executionContext.isValidationOnly() is true; also add a unit test that verifies
"semicolon + extra token" is rejected.
- Around line 3158-3162: In compileNoOp, SqlUtil.fetchNext(lexer) currently
accepts a bare ';' as an argument; change it to validate the fetched token from
the lexer (via SqlUtil.fetchNext or by inspecting lexer state) and if the next
token is a semicolon, EOF or otherwise empty/missing argument, throw a
SqlException indicating a missing/expected argument for the PG no-op statements
instead of silently calling compiledQuery.ofSet(); ensure the validation
references the lexer token returned by SqlUtil.fetchNext(lexer) and only call
compiledQuery.ofSet() after a successful non-empty-argument check.
- Around line 3328-3345: The compileSet method currently accepts malformed SET
statements because it doesn't validate presence of name/operator/value or detect
trailing commas; update compileSet to check each SqlUtil.fetchNext(lexer) result
(name token, operator token, first value, and each value after a comma) and
throw a SqlException when any required token is null or a semicolon (i.e.,
missing name/operator/value or trailing comma). Specifically, in compileSet,
validate the initial tok for name, then validate the operator tok (must not be
null/semicolon), then validate the first value tok, and inside the while loop
ensure the token after a comma is present and valid before continuing; only call
lexer.unparseLast() when the loop was stopped by a non-null non-comma token, and
include clear error messages referencing the offending part (e.g., "missing
value after ',' in SET" or "missing operator/value in SET").

In `@core/src/test/java/io/questdb/test/griffin/engine/SqlCompilerImplTest.java`:
- Around line 7190-7200: The test testTrailingContentAfterSelectRejected
currently triggers the "keyword-as-identifier" path; change the exercised SQL to
be a syntactically complete SELECT followed by extra tokens so the
trailing-token validator fails (for example use "select x from tab where x=0
extra" instead of "select x from tab select x from tab"), and update the
assertExceptionNoLeakCheck call (the SQL string, expected error position and
expected message) to assert the trailing-token validation error from the
validator invoked after compile instead of the keyword-as-identifier message.

@mtopolnik Here's a detailed review of the PR:

1. Critical: SET TIME ZONE will break

The compileSet method assumes the grammar SET [SESSION | LOCAL] name { = | TO } value [, ...]. But PostgreSQL's SET TIME ZONE 'UTC' uses a two-word compound parameter name (TIME ZONE), which doesn't fit this grammar. Tracing through:

SET TIME ZONE 'UTC'
         ↑
         compileSet reads "TIME" as parameter name (not SESSION/LOCAL)
         then reads "ZONE" expecting '=' or 'TO' → throws "'=' or 'TO' expected"

Some PG JDBC drivers send SET TIME ZONE during connection initialization. Before this PR, compileSet was a pure no-op that consumed nothing, so any SET statement was silently accepted. Now it will reject this form. A test for SET TIME ZONE 'UTC' would demonstrate the failure.

2. Use the existing SqlException.unexpectedToken() helper

At SqlCompilerImpl.java:399, the trailing content error is manually constructed:

throw SqlException.$(lexer.lastTokenPosition(), "unexpected token [").put(tok).put(']');

But SqlException.java:152 already has a helper for this exact pattern:

public static SqlException unexpectedToken(int position, CharSequence token) {
    return position(position).put("unexpected token [").put(token).put(']');
}

Should use SqlException.unexpectedToken(lexer.lastTokenPosition(), tok) instead.

3. compileSet semicolon handling: inconsistency with compileBegin/compileCommit/compileRollback

In compileSet, when the do-while loop exits on a semicolon, tok != null is true, so unparseLast() is called — pushing the ; back. In compile() (single-query mode), the trailing content check then re-fetches it and accepts it. This is correct.

However, the BEGIN/COMMIT/ROLLBACK methods have the opposite behavior — they consume the semicolon (no unparseLast when isSemicolon(tok) is true), relying on goToQueryEnd()'s initial unparseLast() to recover in batch mode. The inconsistency between how compileSet and compileBegin handle ; is confusing and fragile.

4. Stale comment in compileBatch

SqlCompilerImpl.java:465:

// We've to move lexer because some query handlers don't consume all tokens (e.g. SET )

After this PR, SET now fully consumes its tokens. This comment is stale.

5. Test coverage gaps

Missing happy-path tests:

• BEGIN TRANSACTION, COMMIT TRANSACTION, ROLLBACK TRANSACTION — the new TRANSACTION-keyword consumption is untested
• SET TIME ZONE 'UTC' — common PG statement (will reveal bug #1 above)

Missing error-path tests:

• BEGIN extra_garbage — should be rejected by trailing content check, but no test verifies this
• COMMIT extra_garbage / ROLLBACK extra_garbage — same gap
• CLOSE ALL extra, DISCARD ALL extra, UNLISTEN * extra — only RESET ALL extra is tested for no-op trailing content

6. Latent test fixes are correct

The PR fixes genuinely broken test SQL that was previously silently accepted:

• AlterTableDropPartitionTest.java:917: Stray ) removed — to_timestamp('2020', 'yyyy')) → to_timestamp('2020', 'yyyy')
• MatViewTest.java:1871: Stray ;) removed — full;) → full
• LimitTest.java:1224: Semicolon misused as comma — limit :lo; :hi → limit :lo, :hi

These are valuable finds.

7. Minor: Qodana exclusion scope

The addition to qodana.yaml excludes SqlCompilerImpl.java from the unused inspection at the file level. It would be better to suppress at the specific method/field level with @SuppressWarnings("unused") to avoid masking future dead code in this large file.

8. Zero-GC compliance

The new code is zero-GC safe — all token operations use existing CharSequence references from the lexer, no allocations on the data path. Looks good.

Summary