feat(spansql): support subquery in View Join

[anicoll]

This feature simply adds the ability to parse DDL statements with subquery's as the test validates.
I think it resolved this issue as well

The highlights are the following:

• Added ScalarSubquery type
• Added SQL generation methods
• Updated parseLit() to detect and parse subqueries

Related: #8519

[gemini-code-assist]

Summary of Changes

Hello @anicoll, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the spansql parser's capabilities by introducing support for scalar subqueries within DDL statements, particularly for CREATE VIEW definitions. This change allows the parser to correctly interpret and represent more advanced SQL syntax, ensuring better compatibility and robustness when working with Spanner SQL DDLs that leverage subqueries.

Highlights

• ScalarSubquery Type Introduction: A new ScalarSubquery struct has been added to represent subqueries that return a single value, enhancing the Abstract Syntax Tree (AST) to accurately model more complex SQL constructs.
• SQL Generation for Scalar Subqueries: New methods, SQL() and addSQL(), have been implemented for the ScalarSubquery type. These methods ensure that scalar subqueries are correctly formatted and enclosed in parentheses when generating SQL strings.
• Enhanced Parser Logic: The parseLit() function in the parser has been updated to intelligently detect and parse scalar subqueries. It now 'sniffs' for a 'SELECT' keyword after an opening parenthesis to differentiate between a regular parenthesized expression and a subquery, allowing for proper parsing of DDL statements containing subqueries.
• Comprehensive Test Coverage: A new test case has been added to parser_test.go specifically for a CREATE OR REPLACE VIEW statement that includes a scalar subquery within a JOIN ON clause. This ensures the parser correctly handles and represents such complex DDL statements.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds a valuable feature to support subqueries within VIEW JOIN clauses. The changes to introduce the ScalarSubquery type and its SQL generation are well-implemented. However, there is a critical issue in the parser logic within parser.go that needs to be addressed. The current implementation for handling parenthesized expressions and subqueries does not consume the opening parenthesis, which can lead to infinite recursion or parsing errors. I've provided a detailed comment and a code suggestion to resolve this.

[gemini-code-assist]

This logic for handling parenthesized expressions and scalar subqueries appears to have a critical bug. The opening parenthesis ( is not consumed before p.parseQuery() or p.parseExpr() is called.

1. For a scalar subquery like (SELECT ...), p.sniff("SELECT") will be true, but since the parser's position is still at (, the subsequent call to p.parseQuery() will likely fail as it expects to see a SELECT keyword.
2. For a regular parenthesized expression like (1+2), p.sniff("SELECT") will be false, and execution will fall through to p.parseExpr(). Since the parser is still positioned at (, calling p.parseExpr() will eventually lead to a recursive call to p.parseLit(), which will see ( again, causing infinite recursion.

The fix is to consume the opening parenthesis ( before deciding which kind of expression to parse. The logic for the entire if tok.value == "(" block should be refactored to handle this correctly.

if tok.value == "(" {
		p.next() // Consume "(".

		// Check if this is a scalar subquery.
		tok := p.peek()
		if tok.kind == keyword && strings.EqualFold(tok.value, "SELECT") {
			q, err := p.parseQuery()
			if err != nil {
				return nil, err
			}
			if err := p.expect(")"); err != nil {
				return nil, err
			}
			return ScalarSubquery{Query: q}, nil
		}

		// It's a regular parenthesized expression.
		e, err := p.parseExpr()
		if err != nil {
			return nil, err
		}

[anicoll]

No this is correct, I have just pushed a commit which validates it with tests and added an extra comment.

[anicoll]

@rahul2393 if you could please be so kind as to review this PR too.

[anicoll]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for scalar subqueries by adding a ScalarSubquery type and updating the parser to recognize (SELECT ...) constructs within expressions. The implementation is clean and includes new methods for SQL generation and extensive test cases covering various scenarios, including SELECT lists, WHERE clauses, and CREATE VIEW statements.

My review identified one potential issue: the parser logic for detecting a subquery only checks for the SELECT keyword. This overlooks subqueries that start with a WITH clause (Common Table Expressions), which would cause parsing to fail for such valid queries. I've provided a suggestion to address this.

Overall, this is a great addition that significantly enhances the parser's capabilities. With the suggested fix, it will be even more robust.

[gemini-code-assist]

The current lookahead only checks for SELECT to identify a subquery. However, a subquery can also start with a WITH clause. This will cause parsing to fail for valid scalar subqueries that use common table expressions (CTEs).

To fix this, you should also check for the WITH keyword.

For example, a query like SELECT (WITH t AS (SELECT 1) SELECT * FROM t) would fail to parse correctly.

I'd also recommend adding a test case to TestParseExpr to cover subqueries that start with a WITH clause to prevent future regressions.

Suggested change

	if p.sniff("SELECT") {
	if p.sniff("SELECT") || p.sniff("WITH") {

[anicoll]

WITH is currently not supported and this PR does not aim to resolve it either as it would grow to large.

[anicoll]

@rahul2393 Is there any way I can get this reviewed soon?

[anicoll]

I ran go vet locally (script from workflows) no issues 🤷

[anicoll]

when running vet.sh from the workflows folder these changes were applied

[anicoll]

Thanks @rahul2393 I also looked at the failing test last run.
Seems it was an unrelated test. maybe flaky.

--- FAIL: TestMultiplexSessionWorker (0.07s)

[anicoll]

@sakthivelmanii if you could please have a look at this 🙏