Skip to content

test(sql): verify recursive CTE handling for #12041 (do not merge) - #12153

Draft
deepyaman wants to merge 10 commits into
mainfrom
verify/recursive-cte-12041
Draft

deepyaman wants to merge 10 commits into
mainfrom
verify/recursive-cte-12041

Conversation

@deepyaman

Copy link
Copy Markdown
Collaborator

Verification PR — do not merge. Measures #12041's fix for #11949 (WITH RECURSIVE dropped when ibis rebuilds the WITH clause) across every backend in CI, including cloud backends via ci-run-cloud.

Commits, each run separately through CI:

  1. Tests only, on unfixed main. test_dot_sql_recursive_cte runs four shapes on every backend with no xfail markers, so CI shows raw outcomes. It checks execution only, so backends that don't use the keyword (MSSQL) or treat it as optional (SQLite) aren't failed for no reason. It also adds compile-only BigQuery unit tests and temporarily adds -v to pytest's options for per-test logs.
  2. fix(sql): preserve RECURSIVE keyword when rebuilding WITH clause #12041's fix (cherry-picked from @bhargaviHQ's commit, code only).
  3. The alternative fix: pass recursive= through with_(), and also keep the flag from the compiled ibis expression in add_query_to_expr.

A full report will be posted as a comment once all three runs finish.

🤖 Generated with Claude Code

Verification-only commit for #12041: exercise WITH RECURSIVE through
con.sql(), a filter on top of con.sql(), Table.sql() with a recursive
query, and Table.sql() over a recursive parent, on every backend. The
tests deliberately carry no xfail markers so CI reports the raw state.

Also add compile-only BigQuery unit tests (the dialect from #11949) and
temporarily run pytest with -v so per-test outcomes appear in CI logs
even if a job times out.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
@deepyaman deepyaman added the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 10, 2026
The translate() and add_query_to_expr() methods in SQLGlotCompiler
discarded the With node's recursive flag when reconstructing CTEs.
Capture the flag before the pop and restore it after the reduce.

Cherry-picked from #12041 (code change only; the tests come from the
previous commit).

Fixes #11949
@github-actions github-actions Bot added tests Issues or PRs related to tests bigquery The BigQuery backend labels Oct 10, 2026
@ibis-docs-bot ibis-docs-bot Bot removed the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 10, 2026
@github-actions github-actions Bot added the sql Backends that generate SQL label Oct 10, 2026
@deepyaman deepyaman added the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 10, 2026
@ibis-docs-bot ibis-docs-bot Bot removed the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 10, 2026
Instead of saving and restoring the flag around the rebuild, hand it to
sqlglot's with_(recursive=...) directly. In add_query_to_expr, also keep
the flag from the compiled ibis expression, not just the user's query,
so `con.sql(recursive_query).alias("t").sql("SELECT ... FROM t")` works.

Build ibis's own CTE aliases as TableAlias rather than bare Identifier.
With RECURSIVE set, sqlglot's Presto/Trino generator reads
`cte.args["alias"].columns` on every CTE, which raised AttributeError
on Trino and Athena.

SingleStoreDB overrides add_query_to_expr with a copy of the same logic,
so apply the same changes there.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
@deepyaman deepyaman added the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 10, 2026
@ibis-docs-bot ibis-docs-bot Bot removed the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 10, 2026
@deepyaman

Copy link
Copy Markdown
Collaborator Author

Verification report: WITH RECURSIVE handling (#11949, #12041)

Bottom line: #12041 is the right idea but incomplete. It fixes the cases where the recursive CTE is in the user's query. It misses the case where the recursive CTE comes from the ibis expression passed to Table.sql(). On Trino and Athena it also turns "table not found" into a sqlglot AttributeError. Commit 3 fixes both: every backend that supports this syntax and doesn't need the keyword dropped now passes all four cases. The only exception is BigQuery when the recursive WITH ends up nested (details below).

What ran

Commit Contents Backends BigQuery Athena Databricks
1 7d9467fc4 tests only, on main run run run run
2 08492755b #12041's base.py change, cherry-picked unchanged run run run run
3 35489f658 proposed fix run run run run

test_dot_sql_recursive_cte has no xfail markers, so these are raw outcomes. It checks only that the query runs and returns the right rows. It doesn't assert on the SQL text, so MSSQL (no RECURSIVE keyword) and SQLite (keyword optional) aren't failed for no reason. The cases:

BigQuery also has compile-only unit tests asserting WITH RECURSIVE is in the output. Those run in fork CI without credentials.

Results

Each cell shows main → #12041 → proposed. ✅ pass, ❌ fail, 🔒 infrastructure error, — didn't run (BigQuery hit its 60-minute job timeout first). Where a backend has several jobs (OS/Python versions), they all agreed.

Backend con_sql con_sql_filtered table_sql_recursive_query table_sql_recursive_parent
athena ❌✅✅ ✅✅✅ ❌❌✅ ❌❌✅
bigquery ❌✅✅ —❌❌ ❌✅✅ ❌❌❌
bigquery (compile-only unit) ❌✅✅ ——— ❌✅✅ ❌❌✅
clickhouse ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
databricks 🔒🔒🔒 🔒🔒🔒 🔒🔒🔒 🔒🔒🔒
datafusion ❌✅✅ ✅✅✅ ❌✅✅ ❌❌✅
druid ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
duckdb ❌✅✅ ✅✅✅ ❌✅✅ ❌❌✅
exasol ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
flink ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
impala ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
materialize ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
mssql ✅✅✅ ✅✅✅ ✅✅✅ ✅✅✅
mysql ❌✅✅ ✅✅✅ ❌✅✅ ❌❌✅
oracle ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
polars ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
postgres ❌✅✅ ✅✅✅ ❌✅✅ ❌❌✅
pyspark ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
risingwave ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
singlestoredb ❌❌❌ ❌❌❌ ❌❌❌ ❌❌❌
sqlite ✅✅✅ ✅✅✅ ✅✅✅ ✅✅✅
trino ❌✅✅ ✅✅✅ ❌❌✅ ❌❌✅

Snowflake isn't included because its workflow is disabled. Databricks errored on every test in every run with Invalid access token (same on main).

Findings

  1. fix(sql): preserve RECURSIVE keyword when rebuilding WITH clause #12041 misses the "recursive parent" case. add_query_to_expr only reads the flag from the user's query and drops it from the compiled ibis expression. table_sql_recursive_parent fails with fix(sql): preserve RECURSIVE keyword when rebuilding WITH clause #12041 on DuckDB, Postgres, MySQL, Trino, DataFusion and Athena, and passes with commit 3 (also in the BigQuery unit test).
  2. New: preserving RECURSIVE broke Trino and Athena (AttributeError: 'Identifier' object has no attribute 'columns' with fix(sql): preserve RECURSIVE keyword when rebuilding WITH clause #12041). With RECURSIVE set, sqlglot's Presto/Trino generator runs add_recursive_cte_column_names, which reads .columns on every CTE's alias. ibis built its own CTE aliases as bare Identifiers. Commit 3 wraps them in sge.TableAlias(this=...), the pattern several other ibis compilers already use.
  3. The save/restore isn't needed. sqlglot's with_() takes recursive=, and has at least since 23.6, which covers our >=23.4 floor. Commit 3 passes it straight through.
  4. SingleStoreDB has its own copy of add_query_to_expr. Commit 3 applies the same change there. With the fix, the SingleStoreDB error changes from "counter doesn't exist" to SingleStore's own "unsupported recursive CTE query shape" (see below).
  5. One side effect of the TableAlias change: test_cte_refs_in_topo_order[mssql] needs a snapshot update. sqlglot's T-SQL generator now aliases CTE projections explicitly ([t0].[key] → [t0].[key] AS [key]). The SQL means the same thing, and I checked locally that the other 20 dialect snapshots are unchanged. Nothing else regressed: apart from this one, the set of other failing tests is the same as on main. One Athena re_extract failure showed up only in commit 2's run and looks flaky.

Remaining failures, not caused by this fix

  • BigQuery con_sql_filtered / table_sql_recursive_parent: in both, ibis puts the recursive WITH inside a subquery or CTE body, and the compiled SQL still contains WITH RECURSIVE. The two top-level cases pass, so this looks like BigQuery only accepting recursive WITH at the top level. That's inferred: the job timed out before pytest printed error details. Fixing it would mean moving nested recursive CTEs to the top level, which is out of scope here.
  • ClickHouse: this one is the test's fault. Results stop one step early ([1, 2]) because ClickHouse lets WHERE see SELECT aliases. In SELECT "n" + 1 AS "n" FROM "counter" WHERE "n" < 3, the WHERE checks the incremented value. Dropping the redundant AS "n" should fix it (not yet run in CI).
  • Trino caveat: if the ibis side compiles to SELECT * (e.g. a bare con.table(...)), sqlglot's transform emits "t"("*"), which is invalid. That needs fixing in sqlglot. The tested shapes compile to "t"(x) and work.
  • Recursive CTEs unsupported or failing at the engine level: Polars ("recursive CTEs are not supported"), Materialize (only WITH MUTUALLY RECURSIVE), SingleStoreDB ("unsupported recursive CTE query shape"), PySpark 3.5/4.0 (planning errors), Oracle (ORA-02000: sqlglot emits RECURSIVE, which Oracle doesn't accept), and Druid, Exasol, Flink, Impala, RisingWave (generic engine errors; I didn't confirm each one is a missing feature).

Suggested follow-up for #12041

  • Use commit 3's approach: pass recursive= through with_(), also keep the flag from the ibis expression, wrap aliases in TableAlias, and mirror it in SingleStoreDB. Update the MSSQL snapshot.
  • Adopt these tests, minus the -v change to pyproject.toml. Drop AS "n" from the recursive step for ClickHouse. Add notyet/notimpl markers for the engine-level failures above, and for BigQuery on the two nested cases.

🤖 Generated with Claude Code

Restrict pytest to the tests relevant to #12041 so the BigQuery job
finishes within its 60-minute limit and prints its failure summary, and
so the other backends turn around faster. Jobs that select none of
these tests (doctests, core) will exit with "no tests collected".

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
…ests

ClickHouse lets WHERE refer to SELECT aliases, so in
`SELECT "n" + 1 AS "n" FROM "counter" WHERE "n" < 3` the WHERE tests the
incremented value and recursion stops one step early ([1, 2] instead of
[1, 2, 3]). The recursive term's column names come from the anchor term
anyway, so the alias was redundant.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
Markers follow the errors observed in CI on #12153. Polars, Exasol,
RisingWave, Druid, Flink and Impala don't support recursive CTEs;
Materialize only supports WITH MUTUALLY RECURSIVE; sqlglot's Oracle
dialect emits a RECURSIVE keyword Oracle rejects; SingleStoreDB rejects
the query shape; Spark < 4.1 fails to parse (3.5) or plan (4.0) them.
BigQuery fails only when the recursive WITH ends up nested inside a
subquery or CTE.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
With CTE aliases built as TableAlias, sqlglot's T-SQL generator names
CTE projections explicitly. The SQL is equivalent.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
@deepyaman deepyaman added the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 12, 2026
@ibis-docs-bot ibis-docs-bot Bot removed the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 12, 2026
Temporary, verification-only: run raw recursive CTE placements against
BigQuery so the failure messages explain the two xfailed nested cases.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
@deepyaman deepyaman added the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 12, 2026
@ibis-docs-bot ibis-docs-bot Bot removed the ci-run-cloud Run BigQuery, Snowflake, Databricks, and Athena backend tests label Oct 12, 2026
@deepyaman

Copy link
Copy Markdown
Collaborator Author

Follow-up: subset run, markers, and the BigQuery limitation

New commits on top of the first report:

  • 4f40d0bfc ci (verification only): run only test_dot_sql.py, test_sql.py and the recursive-CTE tests, using -k. BigQuery now finishes in about 4 minutes instead of timing out.
  • 523893305 test: remove the redundant AS "n" from the recursive step. ClickHouse lets WHERE see SELECT aliases, so WHERE "n" < 3 checked the incremented value and stopped one step early. Reproduced in ClickHouse 26.9 via chDB.
  • e2f585ede test: notyet markers, with raises=, based on the errors CI reported.
  • c214cf2c6 test: update the MSSQL test_cte_refs_in_topo_order snapshot ([t0].[key] → [t0].[key] AS [key], same meaning).
  • 9f048b914 diagnostic (temporary): a BigQuery-only probe of where WITH RECURSIVE is accepted.

Results (Backends, BigQuery, Athena)

Outcome Backends
All 4 cases pass DuckDB, PostgreSQL, MySQL, Trino, DataFusion, Athena, ClickHouse, SQLite, MSSQL
2 top-level cases pass, 2 nested cases XFAIL (GoogleBadRequest) BigQuery
All 4 XFAIL with the expected exception Polars, Exasol, RisingWave, Druid, Flink, Impala, Materialize, Oracle, SingleStoreDB, PySpark
Not testable Databricks (Invalid access token), Snowflake (workflow disabled)

Compared with main (commit 1), nothing that passed before fails now, and 27 backend/case combinations newly pass. The full-suite run of the fix (commit 3 in the first report) showed no other regressions apart from the MSSQL snapshot. A compile-only comparison of 6 CTE-heavy expressions in 21 dialects shows the SQL changes only for MSSQL (that same explicit aliasing). Snowflake and Databricks compile identically.

Why the markers say what they do

Backend Error observed in CI
Polars SQLInterfaceError: recursive CTEs are not supported
Exasol Feature not supported: WITH RECURSIVE
RisingWave recursive CTE not supported for to_batch of LogicalRecursiveUnion (risingwavelabs/risingwave#15135)
Druid Unhandled Query Planning Failure
Flink, Impala sqlglot drops RECURSIVE for these dialects ("Recursive CTEs are unsupported"), and then counter isn't found
Materialize parse error at WITH RECURSIVE; it only supports WITH MUTUALLY RECURSIVE
Oracle ORA-02000: sqlglot's Oracle dialect emits RECURSIVE, which Oracle doesn't accept
SingleStoreDB 2730: Unsupported recursive common table expression query shape
PySpark 3.5: ParseException; 4.0: internal planning errors (recursive CTEs arrived in Spark 4.1). The exception type varies, so this marker has no raises=

BigQuery: understood, and not caused by this change

The probe shows exactly what BigQuery accepts:

Placement Result
top-level WITH RECURSIVE ✅
WITH RECURSIVE inside a subquery ❌ 400 WITH RECURSIVE is only allowed at the top level of the SELECT, CREATE TABLE AS SELECT, CREATE VIEW, INSERT, EXPORT DATA statements.
WITH RECURSIVE inside a CTE body ❌ same error
the same query with the recursive CTE moved to the top-level WITH ✅
non-recursive WITH inside a subquery (control) ✅

con.sql(rec).filter(...) and con.sql(rec).alias("t").sql(...) compile to a nested WITH RECURSIVE. The SQL is the same on main, so these failed before this change too. A fix would move a nested recursive WITH up into the top-level clause. The "moved to the top level" row shows BigQuery accepts that, but it needs care around CTE name collisions, so it fits better as a separate issue.

Unrelated: Python 3.10 jobs now fail at collection

Every Python 3.10 job in this run failed while collecting ibis/backends/tests, with FutureWarning: You are using a non-supported Python version (3.10.22) from google.api_core, which filterwarnings = error turns into an error. google.api_core puts Python 3.10's end-of-life on 2026-10-04, and the 3.10 jobs still collected fine on Oct 10, so this most likely affects main too. It needs a filterwarnings entry or dropping 3.10. Every backend also has a 3.13/3.14 job, so the results above aren't affected.

🤖 Generated with Claude Code

sqlglot's `With` node has had a `recursive` property since before our
23.4 floor, so use it instead of reading the raw arg.

Assisted-by: Claude Fable 5.1 <noreply@anthropic.com>
The compile-only tests exercised base compiler behavior but lived in the
BigQuery unit tests with their own copy of the recursive query. Move them
next to the execution test, reuse its query, and run them for every SQL
backend's dialect the way `test_sql.py` does, so they need no connection.

Compilers are loaded by module path because
`ibis.backends.sql.compilers` doesn't export `singlestoredb`.

Assisted-by: Claude Fable 5.1 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bigquery The BigQuery backend sql Backends that generate SQL tests Issues or PRs related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants