Skip to content

feat(parser): Snowflake LATERAL FLATTEN / TABLE() / named args (#483)#492

Merged
ajitpratap0 merged 4 commits intomainfrom
feat/snowflake-lateral-flatten-483
Apr 8, 2026
Merged

feat(parser): Snowflake LATERAL FLATTEN / TABLE() / named args (#483)#492
ajitpratap0 merged 4 commits intomainfrom
feat/snowflake-lateral-flatten-483

Conversation

@ajitpratap0
Copy link
Copy Markdown
Owner

Summary

Adds Snowflake function-call style table references and named-argument calling convention.

What now parses

  • `SELECT value FROM LATERAL FLATTEN(input => array_col)`
  • `SELECT f.v FROM events, LATERAL FLATTEN(input => events.tags) f`
  • `SELECT * FROM TABLE(my_func(1, 2))`
  • `SELECT * FROM IDENTIFIER('my_table')`
  • `SELECT seq4() FROM TABLE(GENERATOR(rowcount => 100))`

Implementation

  • AST: new `NamedArgument` expression node (`name => expr`); new `TableReference.TableFunc *FunctionCall` field for function-call style table refs. `TableReference.Children()` descends into `TableFunc`.
  • Named args in `parseFunctionCall`: when the current token is an identifier followed by `=>`, build a `NamedArgument` instead of a plain expression. Non-gated — shape is unambiguous and also benefits PostgreSQL procedural calls.
  • FROM function-call ref in `parseFromTableReference`: if the parsed qualified name is followed by `(` and the dialect supports it (Snowflake, BigQuery, PostgreSQL), reparse as a function call and stash on `TableReference.TableFunc`.
  • New `supportsTableFunction` helper.

Test plan

  • `TestSnowflakeFromTableFunctions`: 5 shapes — LATERAL FLATTEN (bare and aliased), TABLE(udf), IDENTIFIER(str), GENERATOR with named args
  • `TestNamedArgumentASTShape`: verifies `NamedArgument` node is reachable via `Children()` traversal
  • `go test -race ./pkg/sql/parser/ ./pkg/sql/ast/ ./pkg/gosqlx/` green

Out of scope

Part of #483.

🤖 Generated with Claude Code

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 8, 2026

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

Project Deployment Actions Updated (UTC)
gosqlx-website Ready Ready Preview, Comment Apr 8, 2026 8:10am

@github-actions github-actions bot added parser ast AST node changes labels Apr 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Claude finished @ajitpratap0's task —— View job


Code Review Completed ✅

I've completed a comprehensive review of PR #492 that adds Snowflake LATERAL FLATTEN and named arguments functionality.

Review Checklist:

  • Read and analyze all changed files
  • Check code quality and adherence to project patterns
  • Identify potential bugs or edge cases
  • Evaluate performance considerations
  • Assess security implications
  • Review test coverage adequacy
  • Provide comprehensive feedback

Overall Assessment

This is a well-implemented feature that follows good engineering practices and integrates cleanly with the existing parser architecture. The code quality is high and the changes are focused and appropriate.

✅ Strengths

Code Quality & Architecture

  • Clean integration: New NamedArgument AST node properly implements the Expression interface
  • Proper pooling: Follows existing object pooling patterns for memory efficiency
  • Dialect-gated: Function table parsing is correctly restricted to supporting dialects (Snowflake, BigQuery, PostgreSQL)
  • AST traversal: NamedArgument.Children() correctly implements the visitor pattern
  • Location tracking: Includes proper source position tracking via models.Location

Implementation Details

  • Unambiguous parsing: Named argument detection (identifier =>) is well-designed and non-conflicting
  • Comprehensive support: Handles all target Snowflake patterns (FLATTEN, TABLE, IDENTIFIER, GENERATOR)
  • Existing token reuse: Leverages existing TokenTypeRArrow (=>) token type
  • Error handling: Proper error propagation in parsing functions

Testing

  • Good coverage: Tests verify both successful parsing and AST structure
  • Regression protection: Tests are tagged with issue number ([epic] Snowflake parser gaps from QA sweep #483) for tracking
  • Visitor verification: TestNamedArgumentASTShape ensures proper AST traversal

⚠️ Areas for Improvement

Memory Safety

pkg/sql/parser/window.go:66 - Missing position safety check:

// Current code:
if p.isIdentifier() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {
    
// Consider adding bounds check:
if p.isIdentifier() && !p.isAtEnd() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {

Edge Case Handling

  • Empty named arguments: Consider testing FLATTEN(=>) or FLATTEN(input=>) invalid cases
  • Mixing patterns: No tests for mixing named and positional args: func(1, name => val, 2)
  • Nested functions: Should verify TABLE(func1(arg => func2(x))) parses correctly

Documentation

  • AST field docs: TableReference.TableFunc could benefit from more usage examples
  • Dialect notes: Consider documenting which specific Snowflake functions support named args

🔒 Security Analysis

Low Risk - No security concerns identified:

  • No user input directly executed
  • No SQL injection vectors introduced
  • Proper input validation via parser error handling
  • Memory allocation follows existing safe patterns

🚀 Performance Considerations

Minimal Impact:

  • Adds one lookahead check (peekToken()) in function parsing - negligible cost
  • New AST node follows same pooling patterns as existing nodes
  • Dialect gating prevents unnecessary parsing in non-Snowflake dialects

📋 Testing Recommendations

Consider adding these test cases:

-- Error cases
FLATTEN(=>) -- missing name
FLATTEN(input=>) -- missing value  
FLATTEN(input => 1, 2) -- mixed named/positional

-- Complex nesting
TABLE(my_func(nested => other_func(x)))
FLATTEN(input => (SELECT array_agg(col) FROM t))

Conclusion

This is a solid implementation that I would recommend for approval. The code follows project conventions, includes appropriate tests, and handles the core use cases well. The suggested improvements are minor enhancements rather than critical issues.

The implementation correctly:

  • ✅ Adds named argument syntax without breaking existing functionality
  • ✅ Integrates function table references with proper dialect support
  • ✅ Maintains memory efficiency with existing pooling patterns
  • ✅ Provides good test coverage for the main scenarios

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Claude finished @ajitpratap0's task —— View job


Code Review Complete ✅

I've completed a comprehensive review of PR #492 that adds Snowflake LATERAL FLATTEN and named arguments functionality.

Review Checklist:

  • Read and analyze all changed files
  • Check code quality and adherence to project patterns
  • Identify potential bugs or edge cases
  • Evaluate performance considerations
  • Assess security implications
  • Review test coverage adequacy
  • Provide comprehensive feedback

Overall Assessment

This is a well-implemented feature that follows good engineering practices and integrates cleanly with the existing parser architecture. The code quality is high and the changes are focused and appropriate.

✅ Strengths

Code Quality & Architecture

  • Clean integration: New NamedArgument AST node properly implements the Expression interface with correct TokenLiteral() and Children() methods
  • Proper AST traversal: NamedArgument.Children() correctly includes the value expression for visitor pattern support
  • Dialect-gated: Function table parsing is correctly restricted to supporting dialects (Snowflake, BigQuery, PostgreSQL) via supportsTableFunction()
  • Location tracking: Includes proper source position tracking via models.Location
  • Consistent patterns: Follows existing codebase conventions for AST node structure and parser integration

Implementation Details

  • Unambiguous parsing: Named argument detection (identifier =>) is well-designed and non-conflicting (line 53-54 in window.go)
  • Comprehensive support: Handles all target Snowflake patterns (FLATTEN, TABLE, IDENTIFIER, GENERATOR)
  • Existing token reuse: Leverages existing TokenTypeRArrow (=>) token type efficiently
  • Error handling: Proper error propagation and descriptive error messages in parsing functions
  • TableReference integration: New TableFunc field properly integrated into Children() method (lines 262-264 in ast.go)

Testing

  • Good coverage: Tests verify both successful parsing and AST structure for 5 different SQL patterns
  • Regression protection: Tests are properly tagged with issue number ([epic] Snowflake parser gaps from QA sweep #483) for tracking
  • Visitor verification: TestNamedArgumentASTShape ensures proper AST traversal and node accessibility

⚠️ Areas for Improvement

Memory Safety

pkg/sql/parser/window.go:53-54 - Consider adding bounds checking for peekToken():

// Current code:
if p.isIdentifier() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {
    
// Consider adding safety:
if p.isIdentifier() && !p.isAtEnd() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {

Edge Case Handling

The implementation could benefit from additional test coverage for edge cases:

  • Invalid named arguments: FLATTEN(=>) or FLATTEN(input=>) (missing name/value)
  • Mixed argument patterns: func(1, name => val, 2) (positional after named)
  • Nested function calls: TABLE(func1(arg => func2(x)))
  • Complex expressions: FLATTEN(input => (SELECT array_agg(col) FROM t))

Documentation Enhancement

  • AST field documentation: TableReference.TableFunc could benefit from more comprehensive usage examples in comments
  • Dialect compatibility notes: Consider documenting which specific Snowflake functions support named arguments

🔒 Security Analysis

Low Risk - No security concerns identified:

  • ✅ No user input directly executed without validation
  • ✅ No SQL injection vectors introduced (input goes through parser validation)
  • ✅ Proper input validation via parser error handling
  • ✅ Memory allocation follows existing safe patterns
  • ✅ No sensitive data exposure in new code paths

🚀 Performance Considerations

Minimal Performance Impact:

  • ✅ Adds only one additional lookahead check (peekToken()) in function parsing - negligible cost
  • ✅ New AST node follows same memory patterns as existing nodes
  • ✅ Dialect gating prevents unnecessary parsing overhead in non-Snowflake dialects
  • ✅ No additional allocations beyond standard AST node creation

📋 Test Coverage Assessment

Current Coverage: Good baseline coverage for main scenarios
Recommended Additions:

-- Error handling tests
FLATTEN(=>) -- missing argument name
FLATTEN(input=>) -- missing argument value  
func(input => val, positional) -- mixed argument styles

-- Complex scenarios
TABLE(my_func(nested => other_func(x))) -- nested function calls
FLATTEN(input => (SELECT array_agg(col) FROM t)) -- subquery arguments

🎯 Conclusion

This is a solid, production-ready implementation that I recommend for approval. The code follows project conventions, includes appropriate tests, and handles the core use cases effectively. The suggested improvements are minor enhancements rather than critical blocking issues.

Key Achievements:

  • ✅ Adds named argument syntax without breaking existing functionality
  • ✅ Integrates function table references with proper dialect support
  • ✅ Maintains code quality and follows established patterns
  • ✅ Provides good test coverage for primary use cases
  • ✅ Includes proper error handling and validation

The implementation correctly addresses the requirements in issue #483 and provides a solid foundation for Snowflake's advanced table function features.


@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Claude finished @ajitpratap0's task —— View job


Code Review Complete ✅

Review Checklist:

  • Read and analyze all changed files
  • Check code quality and adherence to project patterns
  • Identify potential bugs or edge cases
  • Evaluate performance considerations
  • Assess security implications
  • Review test coverage adequacy
  • Provide comprehensive feedback

Overall Assessment

This is a well-implemented feature that follows good engineering practices and integrates cleanly with the existing parser architecture. The code quality is high and the changes are focused and appropriate.

✅ Strengths

Code Quality & Architecture

  • Clean integration: New NamedArgument AST node properly implements the Expression interface with correct TokenLiteral() and Children() methods
  • Proper AST traversal: NamedArgument.Children() correctly includes the value expression for visitor pattern support
  • Dialect-gated: Function table parsing is correctly restricted to supporting dialects (Snowflake, BigQuery, PostgreSQL) via supportsTableFunction()
  • Location tracking: Includes proper source position tracking via models.Location
  • Consistent patterns: Follows existing codebase conventions for AST node structure and parser integration

Implementation Details

  • Unambiguous parsing: Named argument detection (identifier =>) is well-designed and non-conflicting (line 53-54 in window.go)
  • Comprehensive support: Handles all target Snowflake patterns (FLATTEN, TABLE, IDENTIFIER, GENERATOR)
  • Existing token reuse: Leverages existing TokenTypeRArrow (=>) token type efficiently
  • Error handling: Proper error propagation and descriptive error messages in parsing functions
  • TableReference integration: New TableFunc field properly integrated into Children() method (lines 262-264 in ast.go)

Testing

  • Good coverage: Tests verify both successful parsing and AST structure for 5 different SQL patterns
  • Regression protection: Tests are properly tagged with issue number ([epic] Snowflake parser gaps from QA sweep #483) for tracking
  • Visitor verification: TestNamedArgumentASTShape ensures proper AST traversal and node accessibility

⚠️ Areas for Improvement

Memory Safety

pkg/sql/parser/window.go:53-54 - Consider adding bounds checking for peekToken():

// Current code:
if p.isIdentifier() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {
    
// Consider adding safety:
if p.isIdentifier() && !p.isAtEnd() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {

Edge Case Handling

The implementation could benefit from additional test coverage for edge cases:

  • Invalid named arguments: FLATTEN(=>) or FLATTEN(input=>) (missing name/value)
  • Mixed argument patterns: func(1, name => val, 2) (positional after named)
  • Nested function calls: TABLE(func1(arg => func2(x)))
  • Complex expressions: FLATTEN(input => (SELECT array_agg(col) FROM t))

Documentation Enhancement

  • AST field documentation: TableReference.TableFunc could benefit from more comprehensive usage examples in comments
  • Dialect compatibility notes: Consider documenting which specific Snowflake functions support named arguments

🔒 Security Analysis

Low Risk - No security concerns identified:

  • ✅ No user input directly executed without validation
  • ✅ No SQL injection vectors introduced (input goes through parser validation)
  • ✅ Proper input validation via parser error handling
  • ✅ Memory allocation follows existing safe patterns
  • ✅ No sensitive data exposure in new code paths

🚀 Performance Considerations

Minimal Performance Impact:

  • ✅ Adds only one additional lookahead check (peekToken()) in function parsing - negligible cost
  • ✅ New AST node follows same memory patterns as existing nodes
  • ✅ Dialect gating prevents unnecessary parsing overhead in non-Snowflake dialects
  • ✅ No additional allocations beyond standard AST node creation

📋 Test Coverage Assessment

Current Coverage: Good baseline coverage for main scenarios
Recommended Additions:

-- Error handling tests
FLATTEN(=>) -- missing argument name
FLATTEN(input=>) -- missing argument value  
func(input => val, positional) -- mixed argument styles

-- Complex scenarios
TABLE(my_func(nested => other_func(x))) -- nested function calls
FLATTEN(input => (SELECT array_agg(col) FROM t)) -- subquery arguments

🎯 Conclusion

This is a solid, production-ready implementation that I recommend for approval. The code follows project conventions, includes appropriate tests, and handles the core use cases effectively. The suggested improvements are minor enhancements rather than critical blocking issues.

Key Achievements:

  • ✅ Adds named argument syntax without breaking existing functionality
  • ✅ Integrates function table references with proper dialect support
  • ✅ Maintains code quality and follows established patterns
  • ✅ Provides good test coverage for primary use cases
  • ✅ Includes proper error handling and validation

The implementation correctly addresses the requirements in issue #483 and provides a solid foundation for Snowflake's advanced table function features.


@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Claude finished @ajitpratap0's task —— View job


Code Review Complete ✅

Review Checklist:

  • Read and analyze all changed files
  • Check code quality and adherence to project patterns
  • Identify potential bugs or edge cases
  • Evaluate performance considerations
  • Assess security implications
  • Review test coverage adequacy
  • Provide comprehensive feedback

Overall Assessment

This is a well-implemented feature that follows good engineering practices and integrates cleanly with the existing parser architecture. The code quality is high and the changes are focused and appropriate.

✅ Strengths

Code Quality & Architecture

  • Clean integration: New NamedArgument AST node properly implements the Expression interface with correct TokenLiteral() and Children() methods
  • Proper AST traversal: NamedArgument.Children() correctly includes the value expression for visitor pattern support
  • Dialect-gated: Function table parsing is correctly restricted to supporting dialects (Snowflake, BigQuery, PostgreSQL) via supportsTableFunction()
  • Location tracking: Includes proper source position tracking via models.Location
  • Consistent patterns: Follows existing codebase conventions for AST node structure and parser integration

Implementation Details

  • Unambiguous parsing: Named argument detection (identifier =>) is well-designed and non-conflicting (line 53-54 in window.go)
  • Comprehensive support: Handles all target Snowflake patterns (FLATTEN, TABLE, IDENTIFIER, GENERATOR)
  • Existing token reuse: Leverages existing TokenTypeRArrow (=>) token type efficiently
  • Error handling: Proper error propagation and descriptive error messages in parsing functions
  • TableReference integration: New TableFunc field properly integrated into Children() method (lines 262-264 in ast.go)

Testing

  • Good coverage: Tests verify both successful parsing and AST structure for 5 different SQL patterns
  • Regression protection: Tests are properly tagged with issue number ([epic] Snowflake parser gaps from QA sweep #483) for tracking
  • Visitor verification: TestNamedArgumentASTShape ensures proper AST traversal and node accessibility

⚠️ Areas for Improvement

Memory Safety

pkg/sql/parser/window.go:53-54 - Consider adding bounds checking for peekToken():

// Current code:
if p.isIdentifier() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {
    
// Consider adding safety:
if p.isIdentifier() && !p.isAtEnd() &&
    p.peekToken().Token.Type == models.TokenTypeRArrow {

Edge Case Handling

The implementation could benefit from additional test coverage for edge cases:

  • Invalid named arguments: FLATTEN(=>) or FLATTEN(input=>) (missing name/value)
  • Mixed argument patterns: func(1, name => val, 2) (positional after named)
  • Nested function calls: TABLE(func1(arg => func2(x)))
  • Complex expressions: FLATTEN(input => (SELECT array_agg(col) FROM t))

Documentation Enhancement

  • AST field documentation: TableReference.TableFunc could benefit from more comprehensive usage examples in comments
  • Dialect compatibility notes: Consider documenting which specific Snowflake functions support named arguments

🔒 Security Analysis

Low Risk - No security concerns identified:

  • ✅ No user input directly executed without validation
  • ✅ No SQL injection vectors introduced (input goes through parser validation)
  • ✅ Proper input validation via parser error handling
  • ✅ Memory allocation follows existing safe patterns
  • ✅ No sensitive data exposure in new code paths

🚀 Performance Considerations

Minimal Performance Impact:

  • ✅ Adds only one additional lookahead check (peekToken()) in function parsing - negligible cost
  • ✅ New AST node follows same memory patterns as existing nodes
  • ✅ Dialect gating prevents unnecessary parsing overhead in non-Snowflake dialects
  • ✅ No additional allocations beyond standard AST node creation

📋 Test Coverage Assessment

Current Coverage: Good baseline coverage for main scenarios
Recommended Additions:

-- Error handling tests
FLATTEN(=>) -- missing argument name
FLATTEN(input=>) -- missing argument value  
func(input => val, positional) -- mixed argument styles

-- Complex scenarios
TABLE(my_func(nested => other_func(x))) -- nested function calls
FLATTEN(input => (SELECT array_agg(col) FROM t)) -- subquery arguments

🎯 Conclusion

This is a solid, production-ready implementation that I recommend for approval. The code follows project conventions, includes appropriate tests, and handles the core use cases effectively. The suggested improvements are minor enhancements rather than critical blocking issues.

Key Achievements:

  • ✅ Adds named argument syntax without breaking existing functionality
  • ✅ Integrates function table references with proper dialect support
  • ✅ Maintains code quality and follows established patterns
  • ✅ Provides good test coverage for primary use cases
  • ✅ Includes proper error handling and validation

The implementation correctly addresses the requirements in issue #483 and provides a solid foundation for Snowflake's advanced table function features.


@ajitpratap0 ajitpratap0 merged commit f480288 into main Apr 8, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ast AST node changes parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant