diff --git a/CHANGELOG.md b/CHANGELOG.md index 9361adad..0d0bccd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 tokens (`VACUUM t junk`, `LISTEN c extra`) are a parse error instead of being folded into the statement text. MySQL / MariaDB / Oracle `ANALYZE TABLE` keeps its as-written handling. +- SQL Server: a delimited `[top]` / `"top"` is a plain name (also in the select + list, where it was read as the TOP clause) and keeps its delimiters on output; + bare `TOP` is rejected as a table, DML target or alias name, as T-SQL reserves it. +- MariaDB `DELETE HISTORY FROM t [PARTITION (p, ...)] [BEFORE SYSTEM_TIME + [TIMESTAMP | TRANSACTION] expr]` parses and round-trips the partition list, the + unit keyword and any point expression (e.g. `NOW() - INTERVAL 1 DAY`), held in + the new `ast.DMLTargetFields.History` (`ast.DeleteHistoryOptions`). - Watch now respects `--dialect`, re-validates changes and formats files in place with the CLI safety checks; MCP parsing accepts a dialect and documents comment preservation. - Correct transpile dialect help, avoid duplicate lint syntax errors, and rewrite @@ -207,6 +214,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 longer read as MariaDB's `ANALYZE `. ### Behavior changes +- SQL Server: unquoted `top` (any case) is now a syntax error where a table + name, INSERT/UPDATE/DELETE/MERGE target, table alias or select-item alias is + expected (`SELECT * FROM top`, `INSERT INTO top ...`); quote it as `[top]`. - Numeric words without whitespace no longer silently become a number plus a column alias. Numeric spelling is retained. Differences from the previous lexer: diff --git a/pkg/formatter/mariadb_delete_history_test.go b/pkg/formatter/mariadb_delete_history_test.go new file mode 100644 index 00000000..257cf5b6 --- /dev/null +++ b/pkg/formatter/mariadb_delete_history_test.go @@ -0,0 +1,42 @@ +package formatter_test + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestMariaDBDeleteHistoryFormatterRoundTrip(t *testing.T) { + for _, sql := range []string{ + `DELETE HISTORY FROM t PARTITION (p0, p1) BEFORE SYSTEM_TIME TRANSACTION 42`, + `DELETE HISTORY FROM t PARTITION (p0) BEFORE SYSTEM_TIME TIMESTAMP NOW() - INTERVAL 1 DAY`, + "DELETE HISTORY FROM t PARTITION (`select`) BEFORE SYSTEM_TIME '2020-01-01' + INTERVAL 1 DAY", + } { + t.Run(sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + lower := ast.CompactStyle() + lower.KeywordCase = ast.KeywordLower + formatted, err := gosqlx.FormatWithDialect(sql, gosqlx.DefaultFormatOptions(), keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + for _, output := range []string{formatted, formatter.FormatAST(tree, ast.CompactStyle()), formatter.FormatAST(tree, ast.ReadableStyle()), formatter.FormatAST(tree, lower)} { + again, err := gosqlx.ParseWithDialect(output, keywords.DialectMariaDB) + if err != nil { + t.Fatalf("reparse %q: %v", output, err) + } + defer ast.ReleaseAST(again) + if got := again.SQL(); got != sql { + t.Fatalf("formatted %q reparsed to %q, want %q", output, got, sql) + } + } + }) + } +} diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index e9631b36..ce641957 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -596,10 +596,30 @@ func renderDelete(d *ast.DeleteStatement, opts ast.FormatOptions) string { sb.WriteString(" ") } writeDMLTargets(sb, f, x, d.TableName, d.TableNameQuote, d.Alias, d.AliasQuote) + var history ast.DeleteHistoryOptions + if x.History != nil { + history = *x.History + } + if len(history.Partitions) > 0 { + sb.WriteString(" ") + sb.WriteString(f.kw("PARTITION")) + sb.WriteString(" (") + for i, partition := range history.Partitions { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(f.exprSQL(partition)) + } + sb.WriteByte(')') + } if d.BeforeSystemTime != nil { sb.WriteString(" ") sb.WriteString(f.kw("BEFORE SYSTEM_TIME")) sb.WriteString(" ") + if history.BeforeSystemTimeUnit != "" { + sb.WriteString(f.kw(history.BeforeSystemTimeUnit)) + sb.WriteString(" ") + } sb.WriteString(f.exprSQL(d.BeforeSystemTime)) } writeDMLOutput(sb, f, x.Output, x.OutputInto) diff --git a/pkg/sql/ast/ast_dml.go b/pkg/sql/ast/ast_dml.go index 9ea07b43..e05d2dcc 100644 --- a/pkg/sql/ast/ast_dml.go +++ b/pkg/sql/ast/ast_dml.go @@ -50,6 +50,21 @@ type DMLTargetFields struct { Limit Expression // ReturningInto holds the Oracle RETURNING ... INTO :bind targets. ReturningInto []Expression + // History holds the MariaDB DELETE HISTORY partition list and BEFORE + // SYSTEM_TIME unit; nil for every other statement. + History *DeleteHistoryOptions +} + +// DeleteHistoryOptions holds the optional parts of MariaDB's +// DELETE HISTORY FROM t [PARTITION (p, ...)] [BEFORE SYSTEM_TIME [unit] point] +// other than the point itself (DeleteStatement.BeforeSystemTime). It lives in +// DMLTargetFields so the rarely used fields do not grow DeleteStatement. +type DeleteHistoryOptions struct { + // Partitions are the PARTITION (...) names, in source order. + Partitions []*Identifier + // BeforeSystemTimeUnit is "TIMESTAMP" or "TRANSACTION" when written + // before the point, and "" otherwise. + BeforeSystemTimeUnit string } // dmlExtras returns a copy of x, or a zero value when it is nil, so renderers @@ -91,7 +106,7 @@ func (d *DeleteStatement) EnsureExtras() *DMLTargetFields { func (d *DMLTargetFields) IsZero() bool { return len(d.Modifiers) == 0 && d.Top == nil && !d.Only && len(d.Tables) == 0 && len(d.Joins) == 0 && len(d.Output) == 0 && d.OutputInto == nil && - len(d.OrderBy) == 0 && d.Limit == nil && len(d.ReturningInto) == 0 + len(d.OrderBy) == 0 && d.Limit == nil && len(d.ReturningInto) == 0 && d.History == nil } // dmlTargetChildren appends the nodes of d to children. diff --git a/pkg/sql/ast/ast_statements.go b/pkg/sql/ast/ast_statements.go index d81ea794..66492955 100644 --- a/pkg/sql/ast/ast_statements.go +++ b/pkg/sql/ast/ast_statements.go @@ -524,6 +524,8 @@ type DeleteStatement struct { // BeforeSystemTime is the point of MariaDB's DELETE HISTORY FROM t // BEFORE SYSTEM_TIME point; nil when the statement has none. DELETE // HISTORY itself is the HISTORY modifier. + // The PARTITION list and the TIMESTAMP / TRANSACTION unit of the point + // are in Extras.History. BeforeSystemTime Expression // TableNameQuote and AliasQuote record the delimiters of TableName and // Alias when they must be kept (see NameQuote). @@ -549,6 +551,11 @@ func (d DeleteStatement) Children() []Node { for i := range d.From { children = append(children, &d.From[i]) } + if d.Extras != nil && d.Extras.History != nil { + for _, partition := range d.Extras.History.Partitions { + children = append(children, partition) + } + } if d.BeforeSystemTime != nil { children = append(children, d.BeforeSystemTime) } diff --git a/pkg/sql/ast/pool_dml_release.go b/pkg/sql/ast/pool_dml_release.go index e0e4f0e4..af3e25c7 100644 --- a/pkg/sql/ast/pool_dml_release.go +++ b/pkg/sql/ast/pool_dml_release.go @@ -47,6 +47,11 @@ func releaseDMLTargetFields(d *DMLTargetFields, mode releaseMode) { for _, e := range d.ReturningInto { releaseExpression(e, mode) } + if d.History != nil { + for _, partition := range d.History.Partitions { + releaseExpression(partition, mode) + } + } *d = DMLTargetFields{} } diff --git a/pkg/sql/ast/sql.go b/pkg/sql/ast/sql.go index 9975dc25..cd4242ef 100644 --- a/pkg/sql/ast/sql.go +++ b/pkg/sql/ast/sql.go @@ -1578,8 +1578,26 @@ func (d *DeleteStatement) writeSQL(buf *bytes.Buffer) { buf.WriteString("FROM ") } x.writeDMLTargets(buf, d.TableName, d.TableNameQuote, d.Alias, d.AliasQuote) + var history DeleteHistoryOptions + if x.History != nil { + history = *x.History + } + if len(history.Partitions) > 0 { + buf.WriteString(" PARTITION (") + for i, partition := range history.Partitions { + if i > 0 { + buf.WriteString(", ") + } + partition.writeSQL(buf) + } + buf.WriteByte(')') + } if d.BeforeSystemTime != nil { buf.WriteString(" BEFORE SYSTEM_TIME ") + if history.BeforeSystemTimeUnit != "" { + buf.WriteString(history.BeforeSystemTimeUnit) + buf.WriteByte(' ') + } writeExpr(buf, d.BeforeSystemTime) } writeDMLOutput(buf, x.Output, x.OutputInto) diff --git a/pkg/sql/parser/ddl.go b/pkg/sql/parser/ddl.go index c64b09c5..c01ee846 100644 --- a/pkg/sql/parser/ddl.go +++ b/pkg/sql/parser/ddl.go @@ -202,7 +202,7 @@ func (p *Parser) parseCreateTable(temporary bool) (*ast.CreateTableStatement, er } // Parse table name (supports schema.table qualification and double-quoted identifiers) - createTableName, createTableQuote, err := p.parseQualifiedNameQuoted() + createTableName, createTableQuote, err := p.parseQualifiedNameWithTableRules(true) if err != nil { return nil, p.qualifiedNameError(err, "table name") } diff --git a/pkg/sql/parser/dml_common.go b/pkg/sql/parser/dml_common.go index d8180ac8..effe8372 100644 --- a/pkg/sql/parser/dml_common.go +++ b/pkg/sql/parser/dml_common.go @@ -155,6 +155,9 @@ func (p *Parser) isDMLOutputKeywordAt() bool { // isDMLTargetAlias reports whether the current token is a bare alias after // an UPDATE/DELETE target name rather than the clause that follows it. func (p *Parser) isDMLTargetAlias() bool { + if p.isReservedTopName() { + return false + } t := p.currentToken.Token.Type return (t == models.TokenTypeIdentifier || t == models.TokenTypeDoubleQuotedString) && p.isDMLTargetAliasAt() } @@ -190,7 +193,7 @@ func (p *Parser) parseDMLTargetName(name *string, nameQuote *ast.NameQuote, alia *name, *nameQuote = n, q if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() { + if !p.isIdentifier() || p.isReservedTopName() { return p.expectedError("alias after AS") } *alias, *aliasQuote = p.currentToken.Token.Value, p.currentNameQuote() diff --git a/pkg/sql/parser/dml_delete.go b/pkg/sql/parser/dml_delete.go index c3c7bfc5..a700d558 100644 --- a/pkg/sql/parser/dml_delete.go +++ b/pkg/sql/parser/dml_delete.go @@ -179,7 +179,8 @@ func (p *Parser) parseDeleteSources(dst *[]ast.TableReference, joinsDst *[]ast.J } // parseDeleteHistory parses the rest of MariaDB's DELETE HISTORY FROM table -// [BEFORE SYSTEM_TIME point]; the current token is HISTORY. HISTORY is kept as +// [PARTITION (partition_list)] [BEFORE SYSTEM_TIME [TIMESTAMP | TRANSACTION] expr]; +// the current token is HISTORY. HISTORY is kept as // a statement modifier. func (p *Parser) parseDeleteHistory(stmt *ast.DeleteStatement, x *ast.DMLTargetFields) (ast.Statement, error) { x.Modifiers = append(x.Modifiers, "HISTORY") @@ -190,13 +191,43 @@ func (p *Parser) parseDeleteHistory(stmt *ast.DeleteStatement, x *ast.DMLTargetF return nil, err } stmt.TableName, stmt.TableNameQuote = name, quote + var history ast.DeleteHistoryOptions + if p.isType(models.TokenTypePartition) { + p.advance() // Consume PARTITION + if !p.matchType(models.TokenTypeLParen) { + return nil, p.expectedError("(") + } + for { + if !p.isIdentifier() && !p.isNonReservedKeyword() { + return nil, p.expectedError("partition name") + } + partition := p.parseColumnName() + if partition == nil { + return nil, p.expectedError("partition name") + } + history.Partitions = append(history.Partitions, partition) + if !p.matchType(models.TokenTypeComma) { + break + } + } + if !p.matchType(models.TokenTypeRParen) { + return nil, p.expectedError(")") + } + } if p.isUnquotedWord("BEFORE") && p.peekValueIs(1, "SYSTEM_TIME") { p.advance() // Consume BEFORE p.advance() // Consume SYSTEM_TIME - if stmt.BeforeSystemTime, err = p.parseTemporalPointExpression(); err != nil { + if p.isUnquotedWord("TIMESTAMP") || p.isUnquotedWord("TRANSACTION") { + history.BeforeSystemTimeUnit = strings.ToUpper(p.currentToken.Token.Value) + p.advance() + } + if stmt.BeforeSystemTime, err = p.parseExpression(); err != nil { return nil, err } } + if len(history.Partitions) > 0 || history.BeforeSystemTimeUnit != "" { + x.History = &history + } stmt.Extras = finishDMLExtras(x) return stmt, nil } diff --git a/pkg/sql/parser/dml_merge.go b/pkg/sql/parser/dml_merge.go index 91afa6e8..2327b988 100644 --- a/pkg/sql/parser/dml_merge.go +++ b/pkg/sql/parser/dml_merge.go @@ -50,7 +50,7 @@ func (p *Parser) parseMergeStatement() (ast.Statement, error) { // Parse optional target alias (AS alias or just alias) if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() && !p.isNonReservedKeyword() { + if p.isReservedTopName() || (!p.isIdentifier() && !p.isNonReservedKeyword()) { return nil, p.expectedError("target alias after AS") } stmt.TargetAlias = p.currentToken.Token.Value @@ -89,7 +89,7 @@ func (p *Parser) parseMergeStatement() (ast.Statement, error) { // Parse optional source alias if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() && !p.isNonReservedKeyword() { + if p.isReservedTopName() || (!p.isIdentifier() && !p.isNonReservedKeyword()) { return nil, p.expectedError("source alias after AS") } stmt.SourceAlias = p.currentToken.Token.Value diff --git a/pkg/sql/parser/mariadb_delete_history_test.go b/pkg/sql/parser/mariadb_delete_history_test.go new file mode 100644 index 00000000..2a02fd20 --- /dev/null +++ b/pkg/sql/parser/mariadb_delete_history_test.go @@ -0,0 +1,101 @@ +package parser_test + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestMariaDBDeleteHistoryRoundTrip(t *testing.T) { + cases := []string{ + `DELETE HISTORY FROM t`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01 00:00:00'`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TIMESTAMP '2020-01-01 00:00:00'`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME NOW() - INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME CURRENT_TIMESTAMP - INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01' + INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TIMESTAMP NOW() - INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TIMESTAMP '2020-01-01' + INTERVAL 1 DAY`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TRANSACTION 42`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME @ts`, + `DELETE HISTORY FROM db.t BEFORE SYSTEM_TIME '2020-01-01'`, + `DELETE HISTORY FROM t PARTITION (p0, p1)`, + `DELETE HISTORY FROM t PARTITION (p0) BEFORE SYSTEM_TIME '2020-01-01'`, + "DELETE HISTORY FROM t PARTITION (`select`)", + `delete history from t before system_time transaction 7`, + `DELETE FROM history WHERE a = 1`, + `SELECT * FROM t FOR SYSTEM_TIME AS OF TIMESTAMP '2020-01-01'`, + } + for _, sql := range cases { + t.Run(sql, func(t *testing.T) { + want := sql + if strings.HasPrefix(sql, "delete ") { + want = `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TRANSACTION 7` + } + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + if got := tree.SQL(); got != want { + t.Fatalf("SQL() = %q, want %q", got, want) + } + formatted, err := gosqlx.FormatWithDialect(sql, gosqlx.DefaultFormatOptions(), keywords.DialectMariaDB) + if err != nil { + t.Fatal(err) + } + for _, output := range []string{tree.SQL(), formatted} { + again, err := gosqlx.ParseWithDialect(output, keywords.DialectMariaDB) + if err != nil { + t.Fatalf("reparse %q: %v", output, err) + } + defer ast.ReleaseAST(again) + if got := again.SQL(); got != want { + t.Fatalf("round trip = %q, want %q", got, want) + } + } + }) + } +} + +func TestMariaDBDeleteHistoryRejectsInvalid(t *testing.T) { + for _, sql := range []string{ + `DELETE HISTORY FROM t WHERE a = 1`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01' WHERE a = 1`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME '2020-01-01' LIMIT 5`, + `DELETE HISTORY FROM t LIMIT 5`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME`, + `DELETE HISTORY FROM t BEFORE '2020-01-01'`, + `DELETE HISTORY FROM t PARTITION ()`, + `DELETE HISTORY FROM t PARTITION (p0,)`, + `DELETE HISTORY FROM t BEFORE SYSTEM_TIME TRANSACTION`, + } { + t.Run(sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectMariaDB) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil { + t.Fatal("expected syntax error") + } + }) + } +} + +func TestMySQLDeleteHistoryRemainsTarget(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(`DELETE HISTORY FROM t`, keywords.DialectMySQL) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + stmt, ok := tree.Statements[0].(*ast.DeleteStatement) + if !ok { + t.Fatalf("got %T", tree.Statements[0]) + } + if !strings.EqualFold(stmt.TableName, "history") || len(stmt.From) != 1 { + t.Fatalf("expected multi-table DELETE: %#v", stmt) + } +} diff --git a/pkg/sql/parser/name_quote.go b/pkg/sql/parser/name_quote.go index 20738edb..33432b8f 100644 --- a/pkg/sql/parser/name_quote.go +++ b/pkg/sql/parser/name_quote.go @@ -52,7 +52,9 @@ func (p *Parser) delimitedNameMustStayQuoted(name string, quote rune) bool { if quote == 0 { return false } - return ast.DelimitedNameMustStayQuoted(name, p.identifierCaseFolding()) || tokenizer.IsKeyword(name) || + // TOP is a dialect keyword, absent from the tokenizer's global keyword map. + return (p.IsSQLServer() && equalsUpper(name, "TOP")) || + ast.DelimitedNameMustStayQuoted(name, p.identifierCaseFolding()) || tokenizer.IsKeyword(name) || isContextualKeywordName(name) } @@ -142,6 +144,18 @@ func (p *Parser) appendNamePartQuoted(q ast.NameQuote, quote rune) ast.NameQuote // parseQualifiedNameQuoted parses a possibly qualified object name like // parseQualifiedName and also returns how its parts were delimited. func (p *Parser) parseQualifiedNameQuoted() (string, ast.NameQuote, error) { + return p.parseQualifiedNameWithTableRules(false) +} + +// isReservedTopName checks the dialect before inspecting the token text. +func (p *Parser) isReservedTopName() bool { + return p.IsSQLServer() && p.isUnquotedWord("TOP") +} + +func (p *Parser) parseQualifiedNameWithTableRules(table bool) (string, ast.NameQuote, error) { + if table && p.isReservedTopName() { + return "", 0, p.expectedError("table name (TOP is reserved in SQL Server; quote it as [top])") + } if !p.isIdentifier() && !p.isNonReservedKeyword() { return "", 0, p.expectedError("identifier") } @@ -176,7 +190,7 @@ func (p *Parser) parseQualifiedNameQuoted() (string, ast.NameQuote, error) { "", ).WithCause(errQualifiedNameTooLong) } - if !p.isIdentifier() && !p.isNonReservedKeyword() { + if (table && p.isReservedTopName()) || (!p.isIdentifier() && !p.isNonReservedKeyword()) { return "", 0, p.expectedError("identifier after .") } parts[n] = p.currentToken.Token.Value diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index 6f10dbbb..94fa4e2c 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -1400,6 +1400,9 @@ func (p *Parser) nextStartsTableClause() bool { // but NOT contextual clause keywords that would be consumed as aliases by mistake // (e.g. MINUS in Snowflake/Oracle, QUALIFY in Snowflake/BigQuery, WINDOW). func (p *Parser) canBeAlias() bool { + if p.isReservedTopName() { + return false + } // Neither isIdentifier nor isNonReservedKeyword holds at an operand end, // such as the "," or FROM after a select item. if p.atOperandEnd() { diff --git a/pkg/sql/parser/pivot.go b/pkg/sql/parser/pivot.go index abedcdf7..f50642d4 100644 --- a/pkg/sql/parser/pivot.go +++ b/pkg/sql/parser/pivot.go @@ -82,14 +82,14 @@ func (p *Parser) renderPivotValue() string { func (p *Parser) parsePivotAlias(ref *ast.TableReference) { if p.isType(models.TokenTypeAs) { p.advance() // consume AS - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { ref.Alias = p.currentToken.Token.Value ref.AliasQuote = p.currentNameQuote() p.advance() } return } - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { ref.Alias = p.currentToken.Token.Value ref.AliasQuote = p.currentNameQuote() p.advance() diff --git a/pkg/sql/parser/select.go b/pkg/sql/parser/select.go index 4529fc68..e8bb0910 100644 --- a/pkg/sql/parser/select.go +++ b/pkg/sql/parser/select.go @@ -57,7 +57,7 @@ func (p *Parser) parseSelectStatement() (ast.Statement, error) { // Reject TOP in dialects that use LIMIT/OFFSET or ROWNUM/FETCH FIRST instead. // String comparison is used here because the lexer has no TokenTypeTOP constant; // TOP is emitted as an identifier literal (see parseTopClause comment). - if p.isNonTopDialect() && equalsUpper(p.currentToken.Token.Value, "TOP") { + if p.isNonTopDialect() && p.isUnquotedWord("TOP") { if p.dialect == string(keywords.DialectOracle) { return nil, goerrors.UnsupportedFeatureError( "TOP clause is not supported in Oracle; use ROWNUM or FETCH FIRST … ROWS ONLY instead", @@ -441,16 +441,16 @@ func (p *Parser) isNonTopDialect() bool { // Note: "TOP" is detected via a string comparison rather than a dedicated token-type // constant because the lexer does not define a TokenTypeTOP - it tokenises TOP as a // plain identifier/keyword literal. A future lexer enhancement could introduce -// models.TokenTypeTop and replace the equalsUpper check below. +// models.TokenTypeTop and replace the word check below. func (p *Parser) parseTopClause() (*ast.TopClause, error) { switch p.dialect { case string(keywords.DialectSQLServer): - if !equalsUpper(p.currentToken.Token.Value, "TOP") { + if !p.isUnquotedWord("TOP") { return nil, nil } case string(keywords.DialectSnowflake): // Snowflake TOP ; a column named top is left alone. - if !equalsUpper(p.currentToken.Token.Value, "TOP") || p.peekToken().Token.Type != models.TokenTypeNumber { + if !p.isUnquotedWord("TOP") || p.peekToken().Token.Type != models.TokenTypeNumber { return nil, nil } default: diff --git a/pkg/sql/parser/select_subquery.go b/pkg/sql/parser/select_subquery.go index 2d23b3a1..05ec905e 100644 --- a/pkg/sql/parser/select_subquery.go +++ b/pkg/sql/parser/select_subquery.go @@ -169,11 +169,11 @@ func (p *Parser) parseFromTableReference() (ast.TableReference, error) { if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() && !p.isDMLReturningKeyword() && !p.isSelectExtClauseStart() && !p.isTableModifierAfterAliasStart() && !p.isTSQLStatementStart() { if p.isType(models.TokenTypeAs) { p.advance() // Consume AS - if !p.isIdentifier() { + if !p.isIdentifier() || p.isReservedTopName() { return tableRef, p.expectedError("alias after AS") } } - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { tableRef.Alias = p.currentToken.Token.Value tableRef.AliasQuote = p.currentNameQuote() p.advance() @@ -362,11 +362,11 @@ func (p *Parser) parseJoinedTableRef(joinType string) (ast.TableReference, error if (p.isIdentifier() || p.isType(models.TokenTypeAs)) && !p.isMariaDBClauseStart() && !p.isPivotKeyword() && !p.isUnpivotKeyword() && !p.isQualifyKeyword() && !p.isMinusSetOp() && !p.isSnowflakeTimeTravelStart() && !p.isSampleKeyword() && !p.isMatchRecognizeKeyword() && !p.isWindowClauseKeyword() && !p.isSettingsKeyword() && !p.isDMLReturningKeyword() && !p.isSelectExtClauseStart() && !p.isTableModifierAfterAliasStart() && !p.isTSQLStatementStart() { if p.isType(models.TokenTypeAs) { p.advance() - if !p.isIdentifier() { + if !p.isIdentifier() || p.isReservedTopName() { return ref, p.expectedError("alias after AS") } } - if p.isIdentifier() { + if p.isIdentifier() && !p.isReservedTopName() { ref.Alias = p.currentToken.Token.Value ref.AliasQuote = p.currentNameQuote() p.advance() diff --git a/pkg/sql/parser/sqlserver_top_reserved_test.go b/pkg/sql/parser/sqlserver_top_reserved_test.go new file mode 100644 index 00000000..8208c939 --- /dev/null +++ b/pkg/sql/parser/sqlserver_top_reserved_test.go @@ -0,0 +1,133 @@ +package parser_test + +import ( + "strings" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +func TestSQLServerTopReserved(t *testing.T) { + for _, sql := range []string{ + "INSERT INTO top (a) VALUES (1)", "INSERT INTO TOP (a) VALUES (1)", "INSERT INTO Top (a) VALUES (1)", + "SELECT * FROM top", "SELECT * FROM TOP", "DELETE FROM top", "UPDATE top SET a = 1", + "SELECT * FROM t AS top", "SELECT * FROM t top", "SELECT a AS top FROM t", "SELECT 1 top FROM t", "SELECT a top FROM t", + "CREATE TABLE top (a INT)", "SELECT * FROM dbo.top", "MERGE INTO top USING s ON s.a = top.a WHEN MATCHED THEN DELETE", + "SELECT * FROM t PIVOT (SUM(a) FOR q IN ([Q1])) AS top", + "SELECT * FROM t PIVOT (SUM(a) FOR q IN ([Q1])) top", + "SELECT * FROM t JOIN s AS top ON t.a = s.a", "SELECT * FROM t JOIN s top ON t.a = s.a", + "UPDATE t AS top SET a = 1", "DELETE FROM t AS top", + } { + t.Run(sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, keywords.DialectSQLServer) + if tree != nil { + defer ast.ReleaseAST(tree) + } + if err == nil { + t.Fatal("expected syntax error") + } + }) + } +} + +func TestSQLServerTopQuotedRoundTrip(t *testing.T) { + for _, tc := range []struct{ sql, want string }{ + {"SELECT * FROM [top]", "SELECT * FROM [top]"}, {`SELECT * FROM "top"`, `SELECT * FROM "top"`}, + {"SELECT [top] FROM t", "SELECT [top] FROM t"}, {`SELECT "top" FROM t`, `SELECT "top" FROM t`}, + {"SELECT a AS [top] FROM t", "SELECT a AS [top] FROM t"}, {"SELECT * FROM t AS [top]", "SELECT * FROM t [top]"}, + {"INSERT INTO [top] (a) VALUES (1)", "INSERT INTO [top] (a) VALUES (1)"}, {"UPDATE [top] SET a = 1", "UPDATE [top] SET a = 1"}, + {"SELECT top 5 * FROM t", "SELECT TOP 5 * FROM t"}, {"INSERT top (5) INTO t SELECT * FROM s", "INSERT TOP (5) INTO t SELECT * FROM s"}, + {"INSERT Top (5) t (a) VALUES (1)", "INSERT TOP (5) INTO t (a) VALUES (1)"}, {"UPDATE top (5) t SET a = 1", "UPDATE TOP (5) t SET a = 1"}, + {"DELETE top (5) FROM t", "DELETE TOP (5) FROM t"}, {"SELECT t.top FROM t", "SELECT t.top FROM t"}, {"SELECT * FROM [select]", "SELECT * FROM [select]"}, + {"CREATE TABLE t (top INT)", "CREATE TABLE t (top INT)"}, + } { + t.Run(tc.sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(tc.sql, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + got := tree.SQL() + if got != tc.want { + t.Fatalf("got %q want %q", got, tc.want) + } + again, err := gosqlx.ParseWithDialect(got, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(again) + if again.SQL() != got { + t.Fatal("unstable SQL") + } + formatted, err := gosqlx.FormatWithDialect(tc.sql, gosqlx.DefaultFormatOptions(), keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + if strings.Contains(tc.sql, "[top]") && !strings.Contains(formatted, "[top]") { + t.Fatalf("lost brackets: %s", formatted) + } + ft, err := gosqlx.ParseWithDialect(formatted, keywords.DialectSQLServer) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(ft) + if ft.SQL() != got { + t.Fatalf("formatter changed SQL: %s", ft.SQL()) + } + }) + } +} + +func TestTopNamesOtherDialects(t *testing.T) { + for _, dialect := range []keywords.SQLDialect{keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectOracle, keywords.DialectClickHouse, keywords.DialectGeneric, keywords.DialectSnowflake} { + for _, sql := range []string{"INSERT INTO top (a) VALUES (1)", "SELECT * FROM top"} { + t.Run(string(dialect)+sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + if tree.SQL() != sql { + t.Fatalf("SQL() = %q, want %q", tree.SQL(), sql) + } + again, err := gosqlx.ParseWithDialect(tree.SQL(), dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(again) + if tree.SQL() != again.SQL() { + t.Fatal("unstable SQL") + } + formatted, err := gosqlx.FormatWithDialect(sql, gosqlx.DefaultFormatOptions(), dialect) + if err != nil { + t.Fatal(err) + } + ft, err := gosqlx.ParseWithDialect(formatted, dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(ft) + if ft.SQL() != sql { + t.Fatalf("formatter changed SQL: %s", ft.SQL()) + } + }) + } + } +} + +// Quoted TOP must be an expression even in dialects that reject the TOP clause. +func TestDelimitedTopIsNotClause(t *testing.T) { + for _, dialect := range []keywords.SQLDialect{keywords.DialectGeneric, keywords.DialectPostgreSQL, keywords.DialectMySQL, keywords.DialectMariaDB, keywords.DialectSQLite, keywords.DialectOracle, keywords.DialectClickHouse, keywords.DialectSnowflake} { + for _, sql := range []string{`SELECT "top" FROM t`} { + t.Run(string(dialect)+sql, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(sql, dialect) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + }) + } + } +} diff --git a/pkg/sql/parser/statement_boundary.go b/pkg/sql/parser/statement_boundary.go index 5a977577..7f692b4c 100644 --- a/pkg/sql/parser/statement_boundary.go +++ b/pkg/sql/parser/statement_boundary.go @@ -222,5 +222,5 @@ func (p *Parser) parseTableNameQuoted() (string, ast.NameQuote, error) { p.advance() return name, 0, nil } - return p.parseQualifiedNameQuoted() + return p.parseQualifiedNameWithTableRules(true) } diff --git a/pkg/sql/parser/utility_stmt.go b/pkg/sql/parser/utility_stmt.go index f6934630..c86aff11 100644 --- a/pkg/sql/parser/utility_stmt.go +++ b/pkg/sql/parser/utility_stmt.go @@ -994,6 +994,9 @@ func (p *Parser) currentIdentQuote() rune { // follow. Words that start a clause or a set operation are still rejected, // so SELECT a AS FROM t keeps its error. func (p *Parser) isAliasAfterAS() bool { + if p.isReservedTopName() { + return false + } if p.isIdentifier() || p.isNonReservedKeyword() { return true }