diff --git a/cmd/gosqlx/cmd/sql_formatter.go b/cmd/gosqlx/cmd/sql_formatter.go index 159bbc60..47680011 100644 --- a/cmd/gosqlx/cmd/sql_formatter.go +++ b/cmd/gosqlx/cmd/sql_formatter.go @@ -162,7 +162,28 @@ func (f *SQLFormatter) formatSelect(stmt *ast.SelectStatement) error { f.writeNewline() f.writeKeyword("ORDER BY") f.builder.WriteString(" ") - f.formatExpressionList(stmt.OrderBy, ", ") + for i, orderBy := range stmt.OrderBy { + if i > 0 { + f.builder.WriteString(", ") + } + if orderBy.Expression != nil { + if err := f.formatExpression(orderBy.Expression); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to format ORDER BY expression: %v\n", err) + } + } + // Add ASC/DESC + if !orderBy.Ascending { + f.builder.WriteString(" DESC") + } + // Add NULLS FIRST/LAST if specified + if orderBy.NullsFirst != nil { + if *orderBy.NullsFirst { + f.builder.WriteString(" NULLS FIRST") + } else { + f.builder.WriteString(" NULLS LAST") + } + } + } } // LIMIT clause @@ -537,7 +558,26 @@ func (f *SQLFormatter) formatWindowSpec(spec *ast.WindowSpec) error { } f.writeKeyword("ORDER BY") f.builder.WriteString(" ") - f.formatExpressionList(spec.OrderBy, ", ") + for i, orderBy := range spec.OrderBy { + if i > 0 { + f.builder.WriteString(", ") + } + if orderBy.Expression != nil { + if err := f.formatExpression(orderBy.Expression); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to format window ORDER BY expression: %v\n", err) + } + } + if !orderBy.Ascending { + f.builder.WriteString(" DESC") + } + if orderBy.NullsFirst != nil { + if *orderBy.NullsFirst { + f.builder.WriteString(" NULLS FIRST") + } else { + f.builder.WriteString(" NULLS LAST") + } + } + } } if spec.FrameClause != nil { diff --git a/pkg/gosqlx/extract.go b/pkg/gosqlx/extract.go index 75a5e904..f21d068d 100644 --- a/pkg/gosqlx/extract.go +++ b/pkg/gosqlx/extract.go @@ -514,7 +514,9 @@ func (cc *columnCollector) collectFromNode(node ast.Node) { cc.collectFromExpression(n.Having) } for _, ob := range n.OrderBy { - cc.collectFromExpression(ob) + if ob.Expression != nil { + cc.collectFromExpression(ob.Expression) + } } if n.With != nil { cc.collectFromNode(n.With) @@ -670,7 +672,9 @@ func (qcc *qualifiedColumnCollector) collectFromNode(node ast.Node) { qcc.collectFromExpression(n.Having) } for _, ob := range n.OrderBy { - qcc.collectFromExpression(ob) + if ob.Expression != nil { + qcc.collectFromExpression(ob.Expression) + } } if n.With != nil { qcc.collectFromNode(n.With) @@ -833,7 +837,9 @@ func (fc *functionCollector) collectFromNode(node ast.Node) { fc.collectFromExpression(n.Having) } for _, ob := range n.OrderBy { - fc.collectFromExpression(ob) + if ob.Expression != nil { + fc.collectFromExpression(ob.Expression) + } } if n.With != nil { fc.collectFromNode(n.With) diff --git a/pkg/sql/ast/ast.go b/pkg/sql/ast/ast.go index 1d0366ad..08942754 100644 --- a/pkg/sql/ast/ast.go +++ b/pkg/sql/ast/ast.go @@ -113,11 +113,27 @@ func (t *TableReference) statementNode() {} func (t TableReference) TokenLiteral() string { return t.Name } func (t TableReference) Children() []Node { return nil } +// OrderByExpression represents an ORDER BY clause element with direction and NULL ordering +type OrderByExpression struct { + Expression Expression // The expression to order by + Ascending bool // true for ASC (default), false for DESC + NullsFirst *bool // nil = default behavior, true = NULLS FIRST, false = NULLS LAST +} + +func (*OrderByExpression) expressionNode() {} +func (o *OrderByExpression) TokenLiteral() string { return "ORDER BY" } +func (o *OrderByExpression) Children() []Node { + if o.Expression != nil { + return []Node{o.Expression} + } + return nil +} + // WindowSpec represents a window specification type WindowSpec struct { Name string PartitionBy []Expression - OrderBy []Expression + OrderBy []OrderByExpression FrameClause *WindowFrame } @@ -126,7 +142,10 @@ func (w WindowSpec) TokenLiteral() string { return "WINDOW" } func (w WindowSpec) Children() []Node { children := make([]Node, 0) children = append(children, nodifyExpressions(w.PartitionBy)...) - children = append(children, nodifyExpressions(w.OrderBy)...) + for _, orderBy := range w.OrderBy { + orderBy := orderBy // G601: Create local copy to avoid memory aliasing + children = append(children, &orderBy) + } if w.FrameClause != nil { children = append(children, w.FrameClause) } @@ -162,7 +181,7 @@ type SelectStatement struct { GroupBy []Expression Having Expression Windows []WindowSpec - OrderBy []Expression + OrderBy []OrderByExpression Limit *int Offset *int } @@ -195,7 +214,10 @@ func (s SelectStatement) Children() []Node { window := window // G601: Create local copy to avoid memory aliasing children = append(children, &window) } - children = append(children, nodifyExpressions(s.OrderBy)...) + for _, orderBy := range s.OrderBy { + orderBy := orderBy // G601: Create local copy to avoid memory aliasing + children = append(children, &orderBy) + } return children } diff --git a/pkg/sql/ast/dml.go b/pkg/sql/ast/dml.go index 4d5d191e..e2936de0 100644 --- a/pkg/sql/ast/dml.go +++ b/pkg/sql/ast/dml.go @@ -8,7 +8,7 @@ type Select struct { Where Expression GroupBy []Expression Having Expression - OrderBy []Expression + OrderBy []OrderByExpression Limit *int64 Offset *int64 } @@ -29,7 +29,10 @@ func (s Select) Children() []Node { if s.Having != nil { children = append(children, s.Having) } - children = append(children, nodifyExpressions(s.OrderBy)...) + for _, orderBy := range s.OrderBy { + orderBy := orderBy // G601: Create local copy to avoid memory aliasing + children = append(children, &orderBy) + } return children } diff --git a/pkg/sql/ast/dml_test.go b/pkg/sql/ast/dml_test.go index ee7efdcb..841a67ff 100644 --- a/pkg/sql/ast/dml_test.go +++ b/pkg/sql/ast/dml_test.go @@ -75,7 +75,7 @@ func TestSelect(t *testing.T) { From: []TableReference{ {Name: "users"}, }, - OrderBy: []Expression{&Identifier{Name: "created_at"}}, + OrderBy: []OrderByExpression{{Expression: &Identifier{Name: "created_at"}, Ascending: true}}, }, wantLiteral: "SELECT", minChildren: 3, diff --git a/pkg/sql/ast/nodes_test.go b/pkg/sql/ast/nodes_test.go index 92b6bc0d..e53a7a8b 100644 --- a/pkg/sql/ast/nodes_test.go +++ b/pkg/sql/ast/nodes_test.go @@ -343,7 +343,7 @@ func TestWindowSpec(t *testing.T) { windowSpec: &WindowSpec{ Name: "w", PartitionBy: []Expression{&Identifier{Name: "dept"}}, - OrderBy: []Expression{&Identifier{Name: "salary"}}, + OrderBy: []OrderByExpression{{Expression: &Identifier{Name: "salary"}, Ascending: true}}, }, wantLiteral: "WINDOW", wantChildren: 2, diff --git a/pkg/sql/ast/pool.go b/pkg/sql/ast/pool.go index 7bb01339..f413c02e 100644 --- a/pkg/sql/ast/pool.go +++ b/pkg/sql/ast/pool.go @@ -19,7 +19,7 @@ var ( New: func() interface{} { return &SelectStatement{ Columns: make([]Expression, 0, 4), - OrderBy: make([]Expression, 0, 1), + OrderBy: make([]OrderByExpression, 0, 1), } }, } @@ -237,8 +237,10 @@ func PutSelectStatement(stmt *SelectStatement) { for _, col := range stmt.Columns { PutExpression(col) } - for _, expr := range stmt.OrderBy { - PutExpression(expr) + for _, orderBy := range stmt.OrderBy { + if orderBy.Expression != nil { + PutExpression(orderBy.Expression) + } } if stmt.Where != nil { PutExpression(stmt.Where) diff --git a/pkg/sql/parser/nulls_first_last_test.go b/pkg/sql/parser/nulls_first_last_test.go new file mode 100644 index 00000000..c4961ffb --- /dev/null +++ b/pkg/sql/parser/nulls_first_last_test.go @@ -0,0 +1,395 @@ +package parser + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/tokenizer" +) + +// Helper function to parse SQL and return AST (similar to window_functions_test.go) +func parseSQL(t *testing.T, sql string) *ast.AST { + // Get tokenizer from pool + tkz := tokenizer.GetTokenizer() + defer tokenizer.PutTokenizer(tkz) + + // Tokenize SQL + tokens, err := tkz.Tokenize([]byte(sql)) + if err != nil { + t.Fatalf("Failed to tokenize: %v", err) + } + + // Convert tokens for parser - reusing window functions converter which handles all keywords + convertedTokens := convertTokensForWindowFunctions(tokens) + + // Parse tokens + parser := &Parser{} + astObj, err := parser.Parse(convertedTokens) + if err != nil { + t.Fatalf("Failed to parse: %v", err) + } + + return astObj +} + +// TestParser_NullsFirstLast_SelectStatement tests NULLS FIRST/LAST in SELECT ORDER BY +func TestParser_NullsFirstLast_SelectStatement(t *testing.T) { + tests := []struct { + name string + sql string + validateAST func(*testing.T, *ast.AST) + }{ + { + name: "ORDER BY with NULLS FIRST", + sql: "SELECT name FROM users ORDER BY salary NULLS FIRST", + validateAST: func(t *testing.T, astObj *ast.AST) { + if len(astObj.Statements) != 1 { + t.Fatalf("expected 1 statement, got %d", len(astObj.Statements)) + } + sel, ok := astObj.Statements[0].(*ast.SelectStatement) + if !ok { + t.Fatalf("expected *ast.SelectStatement, got %T", astObj.Statements[0]) + } + if len(sel.OrderBy) != 1 { + t.Fatalf("expected 1 ORDER BY expression, got %d", len(sel.OrderBy)) + } + orderBy := sel.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if !*orderBy.NullsFirst { + t.Error("expected NullsFirst to be true") + } + if !orderBy.Ascending { + t.Error("expected Ascending to be true (default)") + } + }, + }, + { + name: "ORDER BY with NULLS LAST", + sql: "SELECT name FROM users ORDER BY salary NULLS LAST", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + orderBy := sel.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if *orderBy.NullsFirst { + t.Error("expected NullsFirst to be false") + } + }, + }, + { + name: "ORDER BY DESC with NULLS FIRST", + sql: "SELECT name FROM users ORDER BY salary DESC NULLS FIRST", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + orderBy := sel.OrderBy[0] + if orderBy.Ascending { + t.Error("expected Ascending to be false") + } + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if !*orderBy.NullsFirst { + t.Error("expected NullsFirst to be true") + } + }, + }, + { + name: "ORDER BY ASC with NULLS LAST", + sql: "SELECT name FROM users ORDER BY salary ASC NULLS LAST", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + orderBy := sel.OrderBy[0] + if !orderBy.Ascending { + t.Error("expected Ascending to be true") + } + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if *orderBy.NullsFirst { + t.Error("expected NullsFirst to be false") + } + }, + }, + { + name: "Multiple ORDER BY columns with mixed NULLS clauses", + sql: "SELECT name FROM users ORDER BY dept NULLS FIRST, salary DESC NULLS LAST", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + if len(sel.OrderBy) != 2 { + t.Fatalf("expected 2 ORDER BY expressions, got %d", len(sel.OrderBy)) + } + + // First column: dept NULLS FIRST + first := sel.OrderBy[0] + if !first.Ascending { + t.Error("expected first column Ascending to be true (default)") + } + if first.NullsFirst == nil { + t.Fatal("expected first column NullsFirst to be non-nil") + } + if !*first.NullsFirst { + t.Error("expected first column NullsFirst to be true") + } + + // Second column: salary DESC NULLS LAST + second := sel.OrderBy[1] + if second.Ascending { + t.Error("expected second column Ascending to be false") + } + if second.NullsFirst == nil { + t.Fatal("expected second column NullsFirst to be non-nil") + } + if *second.NullsFirst { + t.Error("expected second column NullsFirst to be false") + } + }, + }, + { + name: "ORDER BY without NULLS clause (default behavior)", + sql: "SELECT name FROM users ORDER BY salary DESC", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + orderBy := sel.OrderBy[0] + if orderBy.NullsFirst != nil { + t.Errorf("expected NullsFirst to be nil (database default), got %v", *orderBy.NullsFirst) + } + }, + }, + // Note: Complex expressions like "salary * 1.1" in ORDER BY are not yet supported by the parser + // This is a parser limitation, not related to NULLS FIRST/LAST functionality + { + name: "Function call in ORDER BY with NULLS LAST", + sql: "SELECT name FROM users ORDER BY UPPER(name) NULLS LAST", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + orderBy := sel.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if *orderBy.NullsFirst { + t.Error("expected NullsFirst to be false") + } + // Verify expression is a function call + if _, ok := orderBy.Expression.(*ast.FunctionCall); !ok { + t.Errorf("expected FunctionCall, got %T", orderBy.Expression) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + astObj := parseSQL(t, tt.sql) + if tt.validateAST != nil { + tt.validateAST(t, astObj) + } + }) + } +} + +// TestParser_NullsFirstLast_WindowFunctions tests NULLS FIRST/LAST in window function ORDER BY +func TestParser_NullsFirstLast_WindowFunctions(t *testing.T) { + tests := []struct { + name string + sql string + validateAST func(*testing.T, *ast.AST) + }{ + { + name: "Window function with NULLS FIRST", + sql: "SELECT ROW_NUMBER() OVER (ORDER BY salary NULLS FIRST) FROM users", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + if len(sel.Columns) != 1 { + t.Fatalf("expected 1 column, got %d", len(sel.Columns)) + } + funcCall, ok := sel.Columns[0].(*ast.FunctionCall) + if !ok { + t.Fatalf("expected FunctionCall, got %T", sel.Columns[0]) + } + if funcCall.Over == nil { + t.Fatal("expected Over clause to be non-nil") + } + if len(funcCall.Over.OrderBy) != 1 { + t.Fatalf("expected 1 ORDER BY in window spec, got %d", len(funcCall.Over.OrderBy)) + } + orderBy := funcCall.Over.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if !*orderBy.NullsFirst { + t.Error("expected NullsFirst to be true") + } + }, + }, + { + name: "Window function with DESC NULLS LAST", + sql: "SELECT RANK() OVER (ORDER BY salary DESC NULLS LAST) FROM users", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + funcCall := sel.Columns[0].(*ast.FunctionCall) + orderBy := funcCall.Over.OrderBy[0] + if orderBy.Ascending { + t.Error("expected Ascending to be false") + } + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if *orderBy.NullsFirst { + t.Error("expected NullsFirst to be false") + } + }, + }, + { + name: "Window function with PARTITION BY and NULLS FIRST", + sql: "SELECT RANK() OVER (PARTITION BY dept ORDER BY salary DESC NULLS FIRST) FROM employees", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + funcCall := sel.Columns[0].(*ast.FunctionCall) + if funcCall.Over == nil { + t.Fatal("expected Over clause to be non-nil") + } + if len(funcCall.Over.PartitionBy) != 1 { + t.Errorf("expected 1 PARTITION BY expression, got %d", len(funcCall.Over.PartitionBy)) + } + if len(funcCall.Over.OrderBy) != 1 { + t.Fatalf("expected 1 ORDER BY expression, got %d", len(funcCall.Over.OrderBy)) + } + orderBy := funcCall.Over.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if !*orderBy.NullsFirst { + t.Error("expected NullsFirst to be true") + } + }, + }, + { + name: "Window function with multiple ORDER BY and mixed NULLS clauses", + sql: "SELECT DENSE_RANK() OVER (ORDER BY dept NULLS FIRST, salary DESC NULLS LAST) FROM employees", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + funcCall := sel.Columns[0].(*ast.FunctionCall) + if len(funcCall.Over.OrderBy) != 2 { + t.Fatalf("expected 2 ORDER BY expressions, got %d", len(funcCall.Over.OrderBy)) + } + + // First: dept NULLS FIRST + first := funcCall.Over.OrderBy[0] + if first.NullsFirst == nil || !*first.NullsFirst { + t.Error("expected first column NullsFirst to be true") + } + + // Second: salary DESC NULLS LAST + second := funcCall.Over.OrderBy[1] + if second.NullsFirst == nil || *second.NullsFirst { + t.Error("expected second column NullsFirst to be false") + } + }, + }, + { + name: "Window function without NULLS clause", + sql: "SELECT ROW_NUMBER() OVER (ORDER BY salary DESC) FROM users", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + funcCall := sel.Columns[0].(*ast.FunctionCall) + orderBy := funcCall.Over.OrderBy[0] + if orderBy.NullsFirst != nil { + t.Errorf("expected NullsFirst to be nil (database default), got %v", *orderBy.NullsFirst) + } + }, + }, + { + name: "LAG with NULLS LAST in window ORDER BY", + sql: "SELECT LAG(salary, 1) OVER (ORDER BY hire_date NULLS LAST) FROM employees", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + funcCall := sel.Columns[0].(*ast.FunctionCall) + if funcCall.Name != "LAG" { + t.Errorf("expected function name LAG, got %s", funcCall.Name) + } + orderBy := funcCall.Over.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if *orderBy.NullsFirst { + t.Error("expected NullsFirst to be false") + } + }, + }, + { + name: "FIRST_VALUE with complex window spec and NULLS FIRST", + sql: "SELECT FIRST_VALUE(salary) OVER (PARTITION BY dept ORDER BY salary DESC NULLS FIRST ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) FROM employees", + validateAST: func(t *testing.T, astObj *ast.AST) { + sel := astObj.Statements[0].(*ast.SelectStatement) + funcCall := sel.Columns[0].(*ast.FunctionCall) + if funcCall.Name != "FIRST_VALUE" { + t.Errorf("expected function name FIRST_VALUE, got %s", funcCall.Name) + } + orderBy := funcCall.Over.OrderBy[0] + if orderBy.NullsFirst == nil { + t.Fatal("expected NullsFirst to be non-nil") + } + if !*orderBy.NullsFirst { + t.Error("expected NullsFirst to be true") + } + // Verify frame clause exists + if funcCall.Over.FrameClause == nil { + t.Error("expected FrameClause to be non-nil") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + astObj := parseSQL(t, tt.sql) + if tt.validateAST != nil { + tt.validateAST(t, astObj) + } + }) + } +} + +// TestParser_NullsFirstLast_RealWorldQueries tests NULLS FIRST/LAST with realistic queries +// Note: Some advanced SQL features like column aliases (AS keyword) are not yet supported by the parser +func TestParser_NullsFirstLast_RealWorldQueries(t *testing.T) { + tests := []struct { + name string + sql string + description string + }{ + { + name: "Multi-level sorting with mixed NULLS", + sql: `SELECT * + FROM orders + ORDER BY + priority NULLS FIRST, + order_date DESC NULLS LAST, + customer_id NULLS FIRST`, + description: "Complex sorting with different NULL behaviors per column", + }, + // Note: Tests with column aliases (AS keyword) are disabled because the parser doesn't support AS yet + // These would be valuable tests to add once AS keyword support is implemented: + // - E-commerce product ranking with window functions + // - Employee salary analysis with LAG function + // - Sales leaderboard with ROW_NUMBER + // - Window frames with SUM and NULLS ordering + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + astObj := parseSQL(t, tt.sql) + + if astObj == nil { + t.Fatalf("expected non-nil AST for: %s", tt.description) + } + + if len(astObj.Statements) == 0 { + t.Fatalf("expected statements for: %s", tt.description) + } + }) + } +} diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go index baf2f375..b50e806b 100644 --- a/pkg/sql/parser/parser.go +++ b/pkg/sql/parser/parser.go @@ -582,12 +582,30 @@ func (p *Parser) parseWindowSpec() (*ast.WindowSpec, error) { return nil, err } + // Create OrderByExpression with defaults + orderByExpr := ast.OrderByExpression{ + Expression: expr, + Ascending: true, // Default to ASC + NullsFirst: nil, // Default behavior (database-specific) + } + // Check for ASC/DESC after the expression - if p.currentToken.Type == "ASC" || p.currentToken.Type == "DESC" { - p.advance() // Consume ASC/DESC (we don't store it in this simple implementation) + if p.currentToken.Type == "ASC" { + orderByExpr.Ascending = true + p.advance() // Consume ASC + } else if p.currentToken.Type == "DESC" { + orderByExpr.Ascending = false + p.advance() // Consume DESC } - windowSpec.OrderBy = append(windowSpec.OrderBy, expr) + // Check for NULLS FIRST/LAST + nullsFirst, err := p.parseNullsClause() + if err != nil { + return nil, err + } + orderByExpr.NullsFirst = nullsFirst + + windowSpec.OrderBy = append(windowSpec.OrderBy, orderByExpr) if p.currentToken.Type == "," { p.advance() // Consume comma @@ -704,6 +722,26 @@ func (p *Parser) parseFrameBound() (*ast.WindowFrameBound, error) { return bound, nil } +// parseNullsClause parses the optional NULLS FIRST/LAST clause in ORDER BY expressions. +// Returns a pointer to bool indicating null ordering: true for NULLS FIRST, false for NULLS LAST, nil if not specified. +func (p *Parser) parseNullsClause() (*bool, error) { + if p.currentToken.Type == "NULLS" { + p.advance() // Consume NULLS + if p.currentToken.Type == "FIRST" { + t := true + p.advance() // Consume FIRST + return &t, nil + } else if p.currentToken.Type == "LAST" { + f := false + p.advance() // Consume LAST + return &f, nil + } else { + return nil, p.expectedError("FIRST or LAST after NULLS") + } + } + return nil, nil +} + // parseColumnDef parses a column definition func (p *Parser) parseColumnDef() (*ast.ColumnDef, error) { name := p.parseIdent() @@ -1044,22 +1082,45 @@ func (p *Parser) parseSelectStatement() (ast.Statement, error) { } p.advance() // Consume BY - // Parse ORDER BY expression - orderByExpr, err := p.parseExpression() - if err != nil { - return nil, err - } + // Parse order expressions (comma-separated list) + for { + expr, err := p.parseExpression() + if err != nil { + return nil, err + } - // Check for direction (ASC/DESC) - // Note: Direction is handled separately in the actual implementation - if p.currentToken.Type == "DESC" { - p.advance() // Consume DESC - } else if p.currentToken.Type == "ASC" { - p.advance() // Consume ASC - } + // Create OrderByExpression with defaults + orderByExpr := ast.OrderByExpression{ + Expression: expr, + Ascending: true, // Default to ASC + NullsFirst: nil, // Default behavior (database-specific) + } + + // Check for ASC/DESC after the expression + if p.currentToken.Type == "ASC" { + orderByExpr.Ascending = true + p.advance() // Consume ASC + } else if p.currentToken.Type == "DESC" { + orderByExpr.Ascending = false + p.advance() // Consume DESC + } + + // Check for NULLS FIRST/LAST + nullsFirst, err := p.parseNullsClause() + if err != nil { + return nil, err + } + orderByExpr.NullsFirst = nullsFirst + + selectStmt.OrderBy = append(selectStmt.OrderBy, orderByExpr) - // Add ORDER BY to SELECT statement - selectStmt.OrderBy = []ast.Expression{orderByExpr} + // Check for comma (more expressions) + if p.currentToken.Type == "," { + p.advance() // Consume comma + } else { + break + } + } } // Parse LIMIT clause if present diff --git a/pkg/sql/parser/sustained_load_test.go b/pkg/sql/parser/sustained_load_test.go index 68540ef5..3a497403 100644 --- a/pkg/sql/parser/sustained_load_test.go +++ b/pkg/sql/parser/sustained_load_test.go @@ -434,10 +434,16 @@ func TestSustainedLoad_VaryingWorkers(t *testing.T) { t.Skip("Skipping sustained load test in short mode") } - const duration = 5 * time.Second - sql := []byte("SELECT id, name FROM users WHERE active = true") - + // Reduce duration and worker counts when race detection is enabled + // to prevent test timeouts (race detection adds significant overhead) + duration := 5 * time.Second workerCounts := []int{10, 50, 100, 200, 500} + if raceEnabled { + duration = 2 * time.Second // Reduce from 5s to 2s + workerCounts = []int{10, 50, 100} // Reduce worker counts + } + + sql := []byte("SELECT id, name FROM users WHERE active = true") t.Logf("\n=== Varying Workers Performance Test ===") t.Logf("%-10s | %-15s | %-15s | %-15s", "Workers", "Total Ops", "Ops/Sec", "Avg Latency") diff --git a/pkg/sql/parser/window_functions_test.go b/pkg/sql/parser/window_functions_test.go index 3806abdc..807dc9f3 100644 --- a/pkg/sql/parser/window_functions_test.go +++ b/pkg/sql/parser/window_functions_test.go @@ -37,7 +37,8 @@ func convertTokensForWindowFunctions(tokens []models.TokenWithSpan) []token.Toke t.Token.Value == "PRECEDING" || t.Token.Value == "FOLLOWING" || t.Token.Value == "DESC" || t.Token.Value == "ASC" || t.Token.Value == "BETWEEN" || t.Token.Value == "AND" || - t.Token.Value == "ROW" { + t.Token.Value == "ROW" || t.Token.Value == "NULLS" || + t.Token.Value == "FIRST" || t.Token.Value == "LAST" { tokenType = token.Type(t.Token.Value) } else { tokenType = "IDENT"