Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds parsing and modeling of CTEs #1736

Merged
merged 5 commits into from
Feb 7, 2025

Conversation

johnedquinn
Copy link
Member

@johnedquinn johnedquinn commented Feb 5, 2025

Description

  • Adds parsing and ast modeling of the WITH clause.
    • Updates the visitor and AST rewriter
  • This PR also (largely) fixes the grammar related to SELECT as it pertains to the expression tree.
    • Before, exprBagOp and exprSelect were at the top of the expr grammar. This isn't right.
    • Now, expr and selectStatement are distinct variants of dql.
      • selectStatement is now almost identical to the SQL:1999 and SQL:2023 EBNF. I've noted all of the rules and their origin (and where they deviate). Notably, a simpleTable, in PartiQL, can be an arbitrary expression -- which is not allowed in SQL. This allows for the UNION (and other bag ops) of arbitrary expressions. Note that the visitor prohibits non-bag-ops and non-select expressions coupled with the with clause, order by clause, limit clause, and offset clause.
      • There needed to be some slight modifications, such as the introduction of subquery (which is a parenthesized query expression) and the allowance for the direct use of SELECT in a function call (to handle the specification's COLL_AGG(SELECT ...) without the use of additional parentheses).
  • This PR sends an error to the PErrorListener if planning a ExprQuerySet containing a with clause. A future PR should remove this error when implementing the planning/evaluation.

API Changes and Release

There are NO API breaking changes. This feature addition will go in the v1.1.0 release.

This PR adds With and WithListElement. It also adds JoinTypes that allow for LEFT CROSS JOIN, RIGHT CROSS JOIN, FULL CROSS JOIN, and INNER CROSS JOIN. The LEFT CROSS JOIN was already present in the previous implementation. There are parsing conformance tests that also use the remaining.

Upcoming Changes

Before the release of v1.1.0, a follow-up PR will add for planning and evaluation of the WITH clause.

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Adds parsing and ast modeling for WITH clause

Fixes the expression tree to exclude SELECTs

Adds a dedicated subquery node to allow for query expressions
Updates the AST Rewriter for the WITH clause

Adds test for the unsupported feature of planning a with clause
@johnedquinn johnedquinn changed the title [DRAFT] Adds parsing and modeling of CTEs Adds parsing and modeling of CTEs Feb 5, 2025
@johnedquinn johnedquinn requested a review from jpschorr February 5, 2025 21:35
@johnedquinn johnedquinn marked this pull request as ready for review February 5, 2025 21:35
Copy link
Contributor

@jpschorr jpschorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth modeling RECURSIVE now? If nothing else, we could emit an error for now, but we wouldn't need to touch the parser/AST again.

private final List<WithListElement> elements;

/**
* TODO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious when we will 'TODO'? as part of the rest of the CTE work?

*/
@Builder(builderClassName = "Builder")
@EqualsAndHashCode(callSuper = false)
public final class With extends AstNode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a helper constructor for With in org.partiql.ast.Ast?

Adds support for parsing RECURSIVE CTEs

Adds modeling of RECURSIVE in AST

Updates Javadoc TODOs

Adds Ast#with and Ast#withListElement

Adds a TODO for search/cycle
@johnedquinn
Copy link
Member Author

Updated PR to address comments @jpschorr

@johnedquinn johnedquinn requested a review from jpschorr February 7, 2025 16:51
@johnedquinn johnedquinn merged commit 3f0e2e1 into partiql:main Feb 7, 2025
7 checks passed
@johnedquinn johnedquinn deleted the v1-with-clause branch February 7, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants