feat: Add multi-column USING clause support in JOINs (closes #70) (#98)

ajitpratap0 · Ajit Pratap Singh · claude · web-flow · commit cc2d37f925a0 · 2025-11-16T23:10:10.000+05:30
* feat: add stdin/stdout pipeline support (closes #65) Implement comprehensive stdin/stdout pipeline support for all CLI commands (validate, format, analyze, parse) with Unix pipeline conventions and cross-platform compatibility. Features: - Auto-detection: Commands automatically detect piped input - Explicit stdin: Support "-" as stdin marker for all commands - Input redirection: Full support for "< file.sql" syntax - Broken pipe handling: Graceful handling of Unix EPIPE errors - Security: 10MB input limit to prevent DoS attacks - Cross-platform: Works on Unix/Linux/macOS and Windows PowerShell Implementation: - Created stdin_utils.go with pipeline utilities: - IsStdinPipe(): Detects piped input using golang.org/x/term - ReadFromStdin(): Reads from stdin with size limits - GetInputSource(): Unified input detection (stdin/file/direct SQL) - WriteOutput(): Handles stdout and file output with broken pipe detection - DetectInputMode(): Determines input mode based on args and stdin state - ValidateStdinInput(): Security validation for stdin content - Updated all commands with stdin support: - validate.go: Stdin validation with temp file approach - format.go: Stdin formatting (blocks -i flag appropriately) - analyze.go: Stdin analysis with direct content processing - parse.go: Stdin parsing with direct content processing - Dependencies: - Added golang.org/x/term for stdin detection - Testing: - Unit tests: stdin_utils_test.go with comprehensive coverage - Integration tests: pipeline_integration_test.go for real pipeline testing - Manual testing: Validated echo, cat, and redirect operations - Documentation: - Updated README.md with comprehensive pipeline examples - Unix/Linux/macOS and Windows PowerShell examples - Git hooks integration examples Usage Examples: echo "SELECT * FROM users" | gosqlx validate cat query.sql | gosqlx format gosqlx validate - gosqlx format < query.sql cat query.sql | gosqlx format | gosqlx validate Cross-platform: # Unix/Linux/macOS cat query.sql | gosqlx format | tee formatted.sql | gosqlx validate # Windows PowerShell Get-Content query.sql | gosqlx format | Set-Content formatted.sql "SELECT * FROM users" | gosqlx validate Security: - 10MB stdin size limit (MaxStdinSize constant) - Binary data detection (null byte check) - Input validation before processing - Temporary file cleanup in validate command 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve CI failures for PR #97 Fixed 3 critical issues causing all CI builds/tests to fail: 1. Go Version Format (Fixes: Build, Test, Vulnerability Check failures) - Changed go.mod from 'go 1.24.0' (three-part) to 'go 1.24' (two-part) - Three-part format not supported by Go 1.19/1.20 toolchains in CI - Error: 'invalid go version 1.24.0: must match format 1.23' 2. Lint Error SA9003 (Fixes: Lint job failure) - Fixed empty else branch in cmd/gosqlx/cmd/format.go:169-173 - Removed unnecessary else block while preserving same behavior - Staticcheck SA9003: empty branch warning resolved 3. Workflow Go Version Mismatch (Fixes: Security scan failures) - Updated .github/workflows/security.yml to use Go 1.24 - Both GoSec and GovulnCheck jobs now use Go 1.24 - Matches project requirements for golang.org/x/term v0.37.0 All changes maintain backward compatibility and functionality. Related: #65 (stdin/stdout pipeline feature) * fix: update all CI workflows to use Go 1.24 Updated Go version across all GitHub Actions workflows to match go.mod requirements: - .github/workflows/go.yml: Changed build matrix from [1.19, 1.20, 1.21] to [1.24] - .github/workflows/test.yml: Changed test matrix from [1.19, 1.20, 1.21] to [1.24] - .github/workflows/test.yml: Changed benchmark job from 1.21 to 1.24 - .github/workflows/lint.yml: Changed from 1.21 to 1.24 This fixes all remaining CI failures caused by incompatibility between: - Project dependencies (golang.org/x/term v0.37.0) requiring Go 1.24 - Old workflow configurations using Go 1.19-1.21 Related: PR #97, Issue #65 * chore: run go mod tidy to sync dependencies Running go mod tidy updates go.mod format to go 1.24.0 (three-part) which is the standard format for Go 1.24+. This resolves build failures caused by out-of-sync go.mod and go.sum files. Note: Go 1.24 supports both two-part (1.24) and three-part (1.24.0) formats, but go mod tidy standardizes on three-part format. * fix: remove empty if block in validate.go (SA9003) * fix: update staticcheck to latest version for Go 1.24 compatibility * fix: use os.TempDir() for cross-platform test compatibility - Replace hardcoded /tmp/ path with os.TempDir() - Add path/filepath import for filepath.Join - Fixes Windows test failure in TestWriteOutput * feat: add support for multi-column USING clauses in JOINs (Issue #70) Implements SQL-92 compliant multi-column USING support for JOIN operations. Changes: - Modified parser.go to parse comma-separated column lists in USING clause - Added logic to store single columns as Identifier (backward compatibility) - Added logic to store multiple columns as ListExpression - Added comprehensive test coverage for multi-column USING Tests Added: - TestParser_MultiColumnUSING: Tests single and multi-column USING with various JOIN types - TestParser_MultiColumnUSINGEdgeCases: Tests error handling for invalid syntax - TestParser_MultiColumnUSINGWithComplexQueries: Tests integration with WHERE, ORDER BY, LIMIT Examples: - Single column: JOIN table2 USING (id) - Multiple columns: JOIN table2 USING (id, name, category) All existing tests pass with race detection enabled. Maintains full backward compatibility with single-column USING. Generated with Claude Code (https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Ajit Pratap Singh <ajitpratapsingh@Ajits-Mac-mini.local> Co-authored-by: Claude <noreply@anthropic.com>
diff --git a/pkg/sql/parser/join_test.go b/pkg/sql/parser/join_test.go
@@ -555,3 +555,310 @@ func TestParser_JoinTreeLogic(t *testing.T) {
 		}
 	}
 }
+
+// TestParser_MultiColumnUSING tests multi-column USING clause support (Issue #70)
+func TestParser_MultiColumnUSING(t *testing.T) {
+	tests := []struct {
+		name            string
+		sql             string
+		expectedColumns []string
+		wantErr         bool
+	}{
+		{
+			name:            "Single column USING (backward compatibility)",
+			sql:             "SELECT * FROM users JOIN orders USING (id)",
+			expectedColumns: []string{"id"},
+			wantErr:         false,
+		},
+		{
+			name:            "Two column USING",
+			sql:             "SELECT * FROM users JOIN orders USING (id, name)",
+			expectedColumns: []string{"id", "name"},
+			wantErr:         false,
+		},
+		{
+			name:            "Three column USING",
+			sql:             "SELECT * FROM users JOIN orders USING (id, name, category)",
+			expectedColumns: []string{"id", "name", "category"},
+			wantErr:         false,
+		},
+		{
+			name:            "Multiple columns with LEFT JOIN",
+			sql:             "SELECT * FROM users LEFT JOIN orders USING (user_id, account_id)",
+			expectedColumns: []string{"user_id", "account_id"},
+			wantErr:         false,
+		},
+		{
+			name:            "Multiple columns with INNER JOIN",
+			sql:             "SELECT * FROM products INNER JOIN categories USING (category_id, subcategory_id)",
+			expectedColumns: []string{"category_id", "subcategory_id"},
+			wantErr:         false,
+		},
+		{
+			name:            "Four columns USING",
+			sql:             "SELECT * FROM table1 JOIN table2 USING (col1, col2, col3, col4)",
+			expectedColumns: []string{"col1", "col2", "col3", "col4"},
+			wantErr:         false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Get tokenizer from pool
+			tkz := tokenizer.GetTokenizer()
+			defer tokenizer.PutTokenizer(tkz)
+
+			// Tokenize SQL
+			tokens, err := tkz.Tokenize([]byte(tt.sql))
+			if err != nil {
+				t.Fatalf("Failed to tokenize: %v", err)
+			}
+
+			// Convert tokens for parser
+			convertedTokens := convertTokens(tokens)
+
+			// Parse tokens
+			parser := &Parser{}
+			astObj, err := parser.Parse(convertedTokens)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if !tt.wantErr && astObj != nil {
+				defer ast.ReleaseAST(astObj)
+
+				// Verify we have a SELECT statement
+				if len(astObj.Statements) == 0 {
+					t.Fatal("No statements parsed")
+				}
+
+				selectStmt, ok := astObj.Statements[0].(*ast.SelectStatement)
+				if !ok {
+					t.Fatal("Expected SELECT statement")
+				}
+
+				// Verify we have a JOIN
+				if len(selectStmt.Joins) == 0 {
+					t.Fatal("Expected at least one JOIN")
+				}
+
+				join := selectStmt.Joins[0]
+				if join.Condition == nil {
+					t.Fatal("Expected JOIN condition (USING clause)")
+				}
+
+				// Verify the columns
+				if len(tt.expectedColumns) == 1 {
+					// Single column - should be stored as Identifier
+					ident, ok := join.Condition.(*ast.Identifier)
+					if !ok {
+						t.Fatalf("Expected Identifier for single column USING, got %T", join.Condition)
+					}
+					if ident.Name != tt.expectedColumns[0] {
+						t.Errorf("Expected column %s, got %s", tt.expectedColumns[0], ident.Name)
+					}
+				} else {
+					// Multiple columns - should be stored as ListExpression
+					listExpr, ok := join.Condition.(*ast.ListExpression)
+					if !ok {
+						t.Fatalf("Expected ListExpression for multi-column USING, got %T", join.Condition)
+					}
+
+					if len(listExpr.Values) != len(tt.expectedColumns) {
+						t.Fatalf("Expected %d columns, got %d", len(tt.expectedColumns), len(listExpr.Values))
+					}
+
+					// Verify each column
+					for i, expectedCol := range tt.expectedColumns {
+						ident, ok := listExpr.Values[i].(*ast.Identifier)
+						if !ok {
+							t.Fatalf("Column %d: expected Identifier, got %T", i, listExpr.Values[i])
+						}
+						if ident.Name != expectedCol {
+							t.Errorf("Column %d: expected %s, got %s", i, expectedCol, ident.Name)
+						}
+					}
+				}
+			}
+		})
+	}
+}
+
+// TestParser_MultiColumnUSINGEdgeCases tests edge cases for multi-column USING
+func TestParser_MultiColumnUSINGEdgeCases(t *testing.T) {
+	tests := []struct {
+		name          string
+		sql           string
+		expectedError string
+		wantErr       bool
+	}{
+		{
+			name:          "Empty USING clause",
+			sql:           "SELECT * FROM users JOIN orders USING ()",
+			expectedError: "expected column name in USING",
+			wantErr:       true,
+		},
+		{
+			name:          "USING with trailing comma",
+			sql:           "SELECT * FROM users JOIN orders USING (id, name,)",
+			expectedError: "expected column name in USING",
+			wantErr:       true,
+		},
+		{
+			name:          "USING without closing parenthesis",
+			sql:           "SELECT * FROM users JOIN orders USING (id, name",
+			expectedError: "expected ) after USING column list",
+			wantErr:       true,
+		},
+		{
+			name:          "USING without opening parenthesis",
+			sql:           "SELECT * FROM users JOIN orders USING id, name)",
+			expectedError: "expected ( after USING",
+			wantErr:       true,
+		},
+		{
+			name:          "USING with non-identifier",
+			sql:           "SELECT * FROM users JOIN orders USING (id, 123)",
+			expectedError: "expected column name in USING",
+			wantErr:       true,
+		},
+		{
+			name:          "Multiple commas in USING",
+			sql:           "SELECT * FROM users JOIN orders USING (id,, name)",
+			expectedError: "expected column name in USING",
+			wantErr:       true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Get tokenizer from pool
+			tkz := tokenizer.GetTokenizer()
+			defer tokenizer.PutTokenizer(tkz)
+
+			// Tokenize SQL
+			tokens, err := tkz.Tokenize([]byte(tt.sql))
+			if err != nil {
+				// Some tests might fail at tokenization level
+				if tt.wantErr {
+					return // Expected failure
+				}
+				t.Fatalf("Failed to tokenize: %v", err)
+			}
+
+			// Convert tokens for parser
+			convertedTokens := convertTokens(tokens)
+
+			// Parse tokens
+			parser := &Parser{}
+			astObj, err := parser.Parse(convertedTokens)
+
+			if tt.wantErr {
+				if err == nil {
+					if astObj != nil {
+						defer ast.ReleaseAST(astObj)
+					}
+					t.Errorf("Expected error containing '%s', but got no error", tt.expectedError)
+				} else if !containsError(err.Error(), tt.expectedError) {
+					t.Errorf("Expected error containing '%s', got '%s'", tt.expectedError, err.Error())
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Unexpected error: %v", err)
+				}
+				if astObj != nil {
+					defer ast.ReleaseAST(astObj)
+				}
+			}
+		})
+	}
+}
+
+// TestParser_MultiColumnUSINGWithComplexQueries tests multi-column USING in complex scenarios
+func TestParser_MultiColumnUSINGWithComplexQueries(t *testing.T) {
+	tests := []struct {
+		name        string
+		sql         string
+		expectJoins int
+		wantErr     bool
+	}{
+		{
+			name: "Multiple JOINs with multi-column USING",
+			sql: `SELECT * FROM users
+				  JOIN orders USING (user_id, account_id)
+				  JOIN products USING (product_id, category_id)`,
+			expectJoins: 2,
+			wantErr:     false,
+		},
+		{
+			name: "Mixed ON and USING clauses",
+			sql: `SELECT * FROM users u
+				  JOIN orders o USING (user_id, tenant_id)
+				  LEFT JOIN products p ON o.product_id = p.id`,
+			expectJoins: 2,
+			wantErr:     false,
+		},
+		{
+			name: "Multi-column USING with WHERE clause",
+			sql: `SELECT * FROM users
+				  JOIN orders USING (user_id, account_id)
+				  WHERE users.active = true`,
+			expectJoins: 1,
+			wantErr:     false,
+		},
+		{
+			name: "Multi-column USING with ORDER BY and LIMIT",
+			sql: `SELECT * FROM users
+				  JOIN orders USING (user_id, tenant_id)
+				  ORDER BY users.created_at DESC
+				  LIMIT 100`,
+			expectJoins: 1,
+			wantErr:     false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Get tokenizer from pool
+			tkz := tokenizer.GetTokenizer()
+			defer tokenizer.PutTokenizer(tkz)
+
+			// Tokenize SQL
+			tokens, err := tkz.Tokenize([]byte(tt.sql))
+			if err != nil {
+				t.Fatalf("Failed to tokenize: %v", err)
+			}
+
+			// Convert tokens for parser
+			convertedTokens := convertTokens(tokens)
+
+			// Parse tokens
+			parser := &Parser{}
+			astObj, err := parser.Parse(convertedTokens)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if !tt.wantErr && astObj != nil {
+				defer ast.ReleaseAST(astObj)
+
+				// Verify we have a SELECT statement
+				if len(astObj.Statements) == 0 {
+					t.Fatal("No statements parsed")
+				}
+
+				selectStmt, ok := astObj.Statements[0].(*ast.SelectStatement)
+				if !ok {
+					t.Fatal("Expected SELECT statement")
+				}
+
+				// Verify JOIN count
+				if len(selectStmt.Joins) != tt.expectJoins {
+					t.Errorf("Expected %d JOINs, got %d", tt.expectJoins, len(selectStmt.Joins))
+				}
+			}
+		})
+	}
+}
diff --git a/pkg/sql/parser/parser.go b/pkg/sql/parser/parser.go
@@ -905,20 +905,40 @@ func (p *Parser) parseSelectStatement() (ast.Statement, error) {
 					}
 					p.advance()
 
-					// TODO: LIMITATION - Currently only supports single column in USING clause
-					// Future enhancement needed for multi-column support like USING (col1, col2, col3)
-					// This requires parsing comma-separated column list and storing as []Expression
-					// Priority: Medium (Phase 2 enhancement)
-					if p.currentToken.Type != "IDENT" {
-						return nil, p.expectedError("column name in USING")
+					// Parse comma-separated column list for USING clause
+					// Supports both single column: USING (id)
+					// and multi-column: USING (id, name, category)
+					var usingColumns []ast.Expression
+
+					for {
+						// Parse column name
+						if p.currentToken.Type != "IDENT" {
+							return nil, p.expectedError("column name in USING")
+						}
+						usingColumns = append(usingColumns, &ast.Identifier{Name: p.currentToken.Literal})
+						p.advance()
+
+						// Check for comma (more columns)
+						if p.currentToken.Type == "," {
+							p.advance() // Consume comma
+							continue
+						}
+						break
 					}
-					joinCondition = &ast.Identifier{Name: p.currentToken.Literal}
-					p.advance()
 
+					// Check for closing parenthesis
 					if p.currentToken.Type != ")" {
-						return nil, p.expectedError(") after USING column")
+						return nil, p.expectedError(") after USING column list")
 					}
 					p.advance()
+
+					// Store as single identifier for single column (backward compatibility)
+					// or as ListExpression for multiple columns
+					if len(usingColumns) == 1 {
+						joinCondition = usingColumns[0]
+					} else {
+						joinCondition = &ast.ListExpression{Values: usingColumns}
+					}
 				} else if joinType != "NATURAL" {
 					return nil, p.expectedError("ON or USING")
 				}
