sql (fix): Fix SQL generation for POSITION IN subquery

[takezoe]

No description provided.

[amazon-q-developer]

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

[gemini-code-assist]

Hello @takezoe, 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!

Summary of Changes

Hello, gemini-code-assist here! This pull request focuses on fixing an issue within the SQL generation component. Specifically, it addresses the incorrect generation of SQL for the POSITION function when the second argument is a subquery. The changes involve updating the SQL generator to correctly format this specific expression type and adding a new test case to ensure the fix works as expected and prevent regressions.

Highlights

• SQL Generation Fix: The core change is the addition of a specific case in the SQLGenerator to correctly handle Position abstract syntax tree (AST) nodes. This ensures that when a POSITION function is encountered, the generated SQL uses the correct POSITION(substring IN expression) syntax, particularly when the expression is a subquery.
• New Test Case: A new test case has been added to SQLGeneratorTest to specifically verify the scenario where POSITION is used with a subquery. This test confirms that the updated generator produces the expected SQL output for this case.

Changelog

• airframe-sql/src/main/scala/wvlet/airframe/sql/parser/SQLGenerator.scala
  • Added a case for Position expressions in the printExpression function to correctly generate POSITION(substring IN expr) syntax.
• airframe-sql/src/test/scala/wvlet/airframe/sql/parser/SQLGeneratorTest.scala
  • Added a new test print POSITION IN (subquery) to validate the SQL generation for POSITION functions with subquery arguments.

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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

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 issue 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 is currently in preview and 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 to provide feedback.

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.

---

Parse the query tree,
Generate the SQL string,
Fix the missing 'IN'.

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. ↩

[amazon-q-developer]

✅ I finished the code review, and didn't find any security or code quality issues.

[gemini-code-assist]

Code Review

This pull request effectively addresses a bug in SQL generation for POSITION expressions, specifically when the IN clause involves a subquery. The changes are clear, concise, and directly target the issue.

The addition of a specific handler in SQLGenerator.scala for the Position expression ensures that subqueries within the IN part are correctly rendered by using printExpression recursively. This is a solid fix.

The new test case in SQLGeneratorTest.scala is well-chosen, as it directly verifies the scenario with a subquery in the POSITION function, confirming that the generated SQL is now correct.

No external style guide was provided; the code adheres to common Scala conventions and is well-formatted.

Overall, this is a good quality fix. Well done!

Summary of Findings

• SQL Generation Correctness: This PR successfully fixes a bug where POSITION(... IN subquery) expressions were not generating correct SQL. The explicit handling added to SQLGenerator ensures that the subquery part is properly processed by printExpression, leading to valid SQL output. This resolves what would have been a high-severity correctness issue.
• Test Coverage: A new, targeted test case has been added to SQLGeneratorTest.scala. This test specifically covers the POSITION('substring' IN (SELECT ...)) scenario, providing good confidence that the fix works as intended and prevents regressions.

Merge Readiness

The changes in this pull request are well-implemented and effectively address the described issue. The fix is correct, and the accompanying test case provides good validation. Based on this review, the pull request appears ready for merging. As always, ensure any other required reviews or CI checks are completed. I am not authorized to approve pull requests.

[takezoe]

Generated query was like this without this fix:

SELECT xid FROM A WHERE POSITION('str' IN SubQueryExpression(Project[?:? := FunctionCall(max, Id(xid), distinct:false, window:None)](TableScan(name:B, table:default.B, columns:[])),Some(NodeLocation(1,43)))) > 0

Because Position.sqlExpr is used for generating SQL:

airframe/airframe-sql/src/main/scala/wvlet/airframe/sql/model/Expression.scala

Lines 1349 to 1350 in ab3f07e

	override def sqlExpr: String = s"POSITION(${substring.sqlExpr} IN ${string.sqlExpr})"
	override def toString = sqlExpr

[xerial]

LGTM