Skip to content

[CALCITE-7814] Support the SQL-standard CYCLE clause in recursive CTEs - #5294

Merged
rubenada merged 1 commit into
apache:mainfrom
vldpyatkov:calcite-7814
Oct 12, 2026
Merged

rubenada merged 1 commit into
apache:mainfrom
vldpyatkov:calcite-7814

Conversation

@vldpyatkov

@vldpyatkov vldpyatkov commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Jira Link

CALCITE-7814

Changes Proposed

The implemented CYCLE syntax

WITH RECURSIVE
  edges(src, dst) AS (
    VALUES ('A', 'B'), ('B', 'C'), ('C', 'A')
  ),
  walk(node) AS (
    VALUES ('A')
    UNION ALL
    SELECT e.dst
    FROM walk w JOIN edges e ON e.src = w.node
  )
  CYCLE node SET is_cycle TO 'Y' DEFAULT 'N'
    USING cycle_path
SELECT node, is_cycle, cycle_path
FROM walk;

Explicit mark values must be distinct, non-null Boolean literals or compatible single-character string literals. Omitting TO ... DEFAULT ... uses TRUE and FALSE, respectively.
The clause is represented by SqlCycleClause on SqlWithItem. During validation, CycleRewriter rewrites the query to maintain the cycle mark and an ARRAY<ROW<...>> path. A row that closes a cycle is returned but is not expanded further.

During implementation, I found an issue with comparing ARRAY<ROW<...>> values and fixed the comparer to compare their elements by content. This is needed for correct duplicate elimination when the result includes a cycle path column.

@vldpyatkov vldpyatkov changed the title CALCITE-7814 Support the SQL-standard CYCLE clause in recursive CTEs [CALCITE-7814] Support the SQL-standard CYCLE clause in recursive CTEs Sep 25, 2026
@vldpyatkov
vldpyatkov force-pushed the calcite-7814 branch 2 times, most recently from 92a7635 to e7e736e Compare September 27, 2026 17:24
Comment thread core/src/main/codegen/templates/Parser.jj
@rubenada

rubenada commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@vldpyatkov , thanks for this thorough, well-crafted PR. At this point I don't have any further comment, LGTM.
Let's see if we can get any other reviewers opinion on this.

@rubenada

rubenada commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Taking a final look... I have the impression that the current rewrite approach only works if SqlValidator.Config.identifierExpansion is true (I think the existing tests using CalciteAssert / SqlToRelFixture have this flag set to true, but actually the default value is false).

Having a SELECT * ... test with identifierExpansion=false would be required. IIANM, adding these tests to SqlValidatorTest unveils the issue:

  @Test void testCyclePathWithStarInRecursiveTerm() {
    SqlValidatorTestCase.FIXTURE
        .withValidatorIdentifierExpansion(false)
        .withSql("WITH RECURSIVE t(n) AS (VALUES (1) UNION ALL SELECT * FROM t)\n"
            + "CYCLE n SET c TO 'Y' DEFAULT 'N' USING p\n"
            + "SELECT * FROM t")
        .type("RecordType(INTEGER NOT NULL N, CHAR(1) NOT NULL C, "
            + "RecordType(INTEGER NOT NULL EXPR$0) NOT NULL ARRAY NOT NULL P) NOT NULL");
  }

  @Test void testCyclePathWithStarInRecursiveTermIdExpansion() {
    SqlValidatorTestCase.FIXTURE
        .withValidatorIdentifierExpansion(true)
        .withSql("WITH RECURSIVE t(n) AS (VALUES (1) UNION ALL SELECT * FROM t)\n"
            + "CYCLE n SET c TO 'Y' DEFAULT 'N' USING p\n"
            + "SELECT * FROM t")
        .type("RecordType(INTEGER NOT NULL N, CHAR(1) NOT NULL C, "
            + "RecordType(INTEGER NOT NULL EXPR$0) NOT NULL ARRAY NOT NULL P) NOT NULL");
  }

The first one fails, but the second one passes.

A potential solution might be forcing the expansion (independently of the flag) in CycleRewriter#prepareQuery (unverified):

private SqlNode prepareQuery(Reference reference) {
    final SqlSelect step = ((SqlCall) queryBody(item.query)).operand(1);
    final SqlNodeList stepSelectList = step.getSelectList();
    final SqlShuttle shuttle = new DeepCopier() {
      @Override public SqlNode visit(SqlNodeList nodeList) {
        return requireNonNull(
            super.visit(nodeList == stepSelectList ? validator.expandStar(stepSelectList, step, false) : nodeList));
      }
      ...

Comment thread core/src/test/java/org/apache/calcite/test/RecursiveCteTest.java Outdated
Comment thread site/_docs/reference.md
@vldpyatkov

vldpyatkov commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

@rubenada Thanks for catching this! The rewrite now uses the expanded select list saved during validation, even when identifierExpansion is false. I also added tests covering both * and t.* with the flag enabled and disabled.

@rubenada rubenada added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Oct 9, 2026
@rubenada

rubenada commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Thanks @vldpyatkov . In order to prepare the final merge, could you please squash commits into a single one, keeping Jira number and title as final message: [CALCITE-7814] Support the SQL-standard CYCLE clause in recursive CTEs

If no further comments are received, I'll merge this in the next 48h.

@sonarqubecloud

sonarqubecloud Bot commented Oct 9, 2026

Copy link
Copy Markdown

@rubenada
rubenada merged commit 8f27e24 into apache:main Oct 12, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants