[ajitpratap0]

Summary

WP-J (parser grammar edge cases). I reproduced each reported item on origin/main (adc8838):

Item	Result on main	This PR
ClickHouse FROM t FINAL FINAL	Already rejected ("expected a single FINAL modifier")	none
SQL Server INSERT top ... case consistency	TOP clause was consistent in every case; residual bugs were bare top accepted as a name in FROM/INSERT/DELETE/CREATE/aliases but rejected in UPDATE, SELECT [top] FROM t read as the TOP clause, and [top] / "top" losing their delimiters	fixed
MariaDB DELETE HISTORY	Basic form round-tripped; residual bugs were BEFORE SYSTEM_TIME accepting only a primary expression or TIMESTAMP 'lit' (failed on NOW() - INTERVAL 1 DAY and TRANSACTION 42) and no PARTITION (...)	fixed
PG JSON operators vs ^	Already matches PostgreSQL (a -> 'b' ^ 2 = a -> ('b' ^ 2), 2 ^ a -> 'b' = (2 ^ a) -> 'b')	none
Quoted type names	Already preserved in CAST / :: / DDL across dialects	none

Changes:

• SQL Server TOP: isReservedTopName() (a dialect check first, so other dialects pay nothing) now guards table names, DML/MERGE targets, table/join/PIVOT aliases and select-item aliases. parseTopClause and the non-TOP-dialect guard use isUnquotedWord("TOP"). delimitedNameMustStayQuoted keeps delimiters on TOP in SQL Server. TOP is not in tokenizer.IsKeyword, which is why delimiters were dropped before.
• MariaDB DELETE HISTORY: implements the full grammar DELETE HISTORY FROM t [PARTITION (p, ...)] [BEFORE SYSTEM_TIME [TIMESTAMP | TRANSACTION] expr]. The partition list and unit go in a new ast.DMLTargetFields.History *ast.DeleteHistoryOptions, so DeleteStatement keeps its 176 B size class. The fields are rendered by SQL() and pkg/formatter, reachable via Children(), and released and zeroed on pool return. FOR SYSTEM_TIME parsing is unchanged. WHERE and LIMIT stay rejected, as in MariaDB.

Implemented by Codex from a brief. I reviewed it and moved the new DELETE fields out of DeleteStatement after a B/op regression showed up in the benchmark (544 to 592 B/op on BenchmarkParserDelete).

Test Plan

• New tests: pkg/sql/parser/sqlserver_top_reserved_test.go, pkg/sql/parser/mariadb_delete_history_test.go, pkg/formatter/mariadb_delete_history_test.go. They cover parse, SQL(), re-parse stability, formatter output (compact, readable, lower-case) and rejection cases, plus checks that other dialects are unchanged.
• task check: pass, no FAIL lines.
• task test:cbinding: pass.
• Benchmarks: benchstat, 6 interleaved runs each, main vs branch, for SimpleSelect, ComplexSelect, Insert, Delete and ParseBytesWithDialect (pg, mysql). Time geomean -0.07%; B/op and allocs/op identical on every benchmark.

Breaking Changes

• SQL Server: unquoted top (any case) is now a syntax error as a table name, INSERT/UPDATE/DELETE/MERGE target, table alias or select-item alias, as in T-SQL. Quote it as [top]. Other dialects are unchanged.
• New public AST: ast.DMLTargetFields.History and ast.DeleteHistoryOptions. This is additive.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DnbRnTFErhoMJdAjVEeCg2

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 28, 2026 7:19pm UTC

[github-actions]

Claude finished @ajitpratap0's task in 19s —— View job

---

Review of PR #880

I read the diff only. I didn't build or run the tests, so I haven't confirmed the task check and benchmark results from the description.

Overall: The change is clean and well scoped. Putting the new DELETE fields in DMLTargetFields.History keeps DeleteStatement at its size class, and isReservedTopName() checks the dialect first, so other dialects pay almost nothing.

SQL Server TOP

• The guard is applied consistently: table names, DML targets, AS aliases, bare aliases (canBeAlias, isDMLTargetAlias, isAliasAfterAS), and keeping [top] quoted in delimitedNameMustStayQuoted.
• parseQualifiedNameWithTableRules(table bool) is a new function name used by a thin wrapper. Two small things to tidy:
  • parseQualifiedNameWithTableRules has no doc comment, and isReservedTopName has only a one-liner.
  • The table flag also rejects TOP after a . (dbo.top). That matches T-SQL, but the error text "identifier after ." doesn't say why. Using the same "TOP is reserved… quote it as [top]" hint there would help users.
• Quoted column names or qualifiers called top in expressions are unaffected, which looks right. A test for SELECT t.[top] FROM t and SELECT "top" with QUOTED_IDENTIFIER would lock that in.

MariaDB DELETE HISTORY

• Switching from parseTemporalPointExpression to parseExpression fixes NOW() - INTERVAL 1 DAY and TRANSACTION 42.
• The unit is detected by an unquoted-word check on TIMESTAMP/TRANSACTION. That means a column literally named timestamp (BEFORE SYSTEM_TIME timestamp) is parsed as a unit followed by an expression and fails. This is probably an acceptable trade-off, because MariaDB reads it the same way. A test documenting the behaviour would be good.
• Partition names go through parseColumnName(), which may accept qualified names such as a.b. MariaDB partition names are plain identifiers. Consider a single-identifier check, or a rejection test.
• x.History is only set when there are partitions or a unit. When there is neither, History stays nil, so IsZero and the pool zeroing remain correct.
• Release, Children(), SQL() and the formatter are all updated and consistent. The SQL() and formatter render paths duplicate the partition-list logic, which is the existing pattern in this codebase.

Tests: Coverage looks good: parse, SQL round-trip, formatter casing, rejections, and other-dialect checks. I'd add the dbo.top / [top] qualified cases and a PARTITION (a.b) rejection.

Performance and security: I found no concerns. The benchmark numbers in the description show no B/op or allocs/op change.

Verdict: Looks good to merge. The comments above are minor nits.

[github-actions]

Claude finished @ajitpratap0's task in 22s —— View job

---

Review of PR #880 (updated for the latest push)

I read the diff only. I didn't build or run the tests, so I haven't confirmed the task check and benchmark results from the description.

Overall: The change is well scoped and I found no blocking issues.

• isReservedTopName() checks the dialect first, so other dialects pay nothing.
• Putting the new DELETE fields in DMLTargetFields.History keeps DeleteStatement in its size class.

SQL Server TOP

• The guard is applied consistently:
  • table names
  • DML and MERGE targets
  • AS aliases
  • bare aliases (canBeAlias, isDMLTargetAlias)
  • delimitedNameMustStayQuoted
• Error text for dbo.top: the qualified-part check (pkg/sql/parser/name_quote.go, "identifier after .") rejects TOP but doesn't say why. The first-part error includes the "TOP is reserved… quote it as [top]" hint, so reuse it here.
• Doc comment: parseQualifiedNameWithTableRules has no doc comment. A short note on what table changes would help.
• Test gaps: I'd add tests for these cases, to lock in that quoted forms still work:
  • SELECT t.[top] FROM t
  • dbo.[top]
  • "top" under QUOTED_IDENTIFIER
  • dbo.top being rejected

MariaDB DELETE HISTORY

• Switching to parseExpression fixes NOW() - INTERVAL 1 DAY and TRANSACTION 42.
• Partition names: they are parsed with parseColumnName() (pkg/sql/parser/dml_delete.go). That likely accepts qualified names such as PARTITION (a.b), which MariaDB doesn't allow. I'd add a single-identifier check, or at least a rejection test.
• Column named timestamp: BEFORE SYSTEM_TIME timestamp treats timestamp as the unit and then fails on the missing expression. MariaDB reads it the same way, so this is fine, but a test documenting it would help.
• History stays nil when there are no partitions and no unit, so IsZero and pool zeroing stay correct.
• SQL() and the formatter duplicate the partition-list rendering. That follows the existing pattern in this codebase.

Performance and security

No concerns. The benchmark numbers in the description show no B/op or allocs/op change. Nothing here touches input handling in a way that adds risk.

Verdict: Good to merge. The points above are minor and non-blocking.