Skip to content

Fix CTE reference resolution slt tests#21049

Open
jonahgao wants to merge 2 commits intoapache:mainfrom
jonahgao:cte_test
Open

Fix CTE reference resolution slt tests#21049
jonahgao wants to merge 2 commits intoapache:mainfrom
jonahgao:cte_test

Conversation

@jonahgao
Copy link
Member

@jonahgao jonahgao commented Mar 19, 2026

Which issue does this PR close?

N/A

Rationale for this change

PR #19519 introduces new tests to verify that DataFusion won't unexpectedly lookup CTE names from the catalog. It registers a strict SchemaProvider that panics with unexpected table lookups.

async fn table(
&self,
name: &str,
) -> Result<Option<Arc<dyn TableProvider>>, DataFusionError> {
match name {
"orders" => Ok(Some(Arc::clone(&self.orders))),
other => panic!(
"unexpected table lookup: {other}. This maybe indicates a CTE reference was \
incorrectly treated as a catalog table reference."
),
}
}

The problem is that PR #19862 skips registering the strict SchemaProvider and bypasses the unexpected lookup check. Therefore, there tests become meaningless.

What changes are included in this PR?

Re-register the strict SchemaProvider into cte.slt.

Are these changes tested?

Yes.

I also backported these new tests to tag 52.0.0, which does not include the fix in #19519, and they failed as expected.

$ cargo test --profile release-nonlto --test sqllogictests -- cte
   Compiling datafusion-sqllogictest v52.0.0 (/Users/jonah/Work/datafusion/datafusion/sqllogictest)
    Finished `release-nonlto` profile [optimized] target(s) in 19.50s
     Running bin/sqllogictests.rs (/Users/jonah/Work/datafusion/target/release-nonlto/deps/sqllogictests-f9c15c7896eb5af9)
[00:00:00] ####------------------------------------       6/75      "cte.slt"
thread 'tokio-runtime-worker' (486408) panicked at datafusion/sqllogictest/src/test_context.rs:215:22:
unexpected table lookup: barbaz. This maybe indicates a CTE reference was incorrectly treated as a catalog table reference.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Completed 3 test files in 0 seconds                                                                                                                                                          failure in cte.slt for sql with barbaz as (select * from orders) select * from "barbaz";
caused by
External error: task 13 panicked with message "unexpected table lookup: barbaz. This maybe indicates a CTE reference was incorrectly treated as a catalog table reference."
Error: Execution("1 failures")
error: test failed, to rerun pass `-p datafusion-sqllogictest --test sqllogictests`

Are there any user-facing changes?

No.


// Override the default "datafusion" catalog for this test file so that any
// unexpected lookup is caught immediately.
ctx.register_catalog(
Copy link
Member Author

Choose a reason for hiding this comment

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

Register a new schema instead of overriding the default datafusion catalog to avoid impacting other CTE tests.


let file_name = relative_path.file_name().unwrap().to_str().unwrap();
match file_name {
"cte_quoted_reference.slt" => {
Copy link
Member Author

Choose a reason for hiding this comment

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

cte_quoted_reference.slt has been consolidated into cte.slt, and no longer exists.

WITH a AS (SELECT 1), a AS (SELECT 2) SELECT * FROM a;

statement ok
CREATE TABLE orders AS VALUES (1), (2);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should use the orders table from the StrictSchemaProvider, rather than creating a new one


# Reset to default configs
statement ok
set datafusion.sql_parser.enable_ident_normalization = true;
Copy link
Member Author

Choose a reason for hiding this comment

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

Related to #19862 (review)

Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants