test(unparser): share a fetch-parameterized fixture between the bounded and unbounded EXISTS cases

The bounded/unbounded pairs exist to pin that the refusal is gated on the row
bound rather than on the shape of the correlation. That only holds while each
pair's two plans are identical apart from the bound, which copy-paste made a
convention rather than a property — an edit to one side would silently leave the
pair comparing two different shapes.

Parameterize by `fetch`, matching `qualified_build_side_semi_join` alongside it.
The limit is applied only when asked, since `limit(0, None)` still inserts a
`Limit` node.

Also drops a stale claim on the fully-qualified-dialect test: it described the
other two shapes as declining to SQL that "does not bind, which a database
rejects", which is the premise the refusals overturn.

Author: claudespice

datafusion/sql/src/unparser/plan.rs

@@ -2732,9 +2732,8 @@ impl Unparser<'_> {
     /// is a bound this unparser cannot place — and leaving the bound beside the
     /// correlation emits SQL that *binds and runs* while answering from rows
     /// outside the bound. A wrong answer that executes is worse than one that
-    /// does not, so these cost the pushdown instead: the trade
-    /// `derive_row_limited_scope` makes for the limit it scopes, and the one the
-    /// fully-qualified-dialect arm below already made.
+    /// does not, so these cost the pushdown instead — the trade
+    /// `derive_row_limited_scope` makes for the limit it scopes.
     ///
     /// Refusing is not the same as repairing. Each shape below is emitted
     /// correctly only once the correlation's own qualifiers are rewritten to the

datafusion/sql/tests/cases/plan_to_sql.rs

@@ -3201,11 +3201,10 @@ fn qualified_build_side_semi_join(fetch: Option<usize>) -> Result<LogicalPlan> {
 /// name in the correlated predicate, but a derived table's alias is a single
 /// identifier and can only carry the last one, so the bound cannot be scoped.
 ///
-/// This one is refused rather than declined. The other declines in
-/// `exists_scope_name` fall back to SQL that does not bind, which a database
-/// rejects; leaving the bound unscoped *here* would bind and run, silently
-/// answering from rows outside the bound. Losing the pushdown is the cheaper
-/// failure, and it is the trade `derive_row_limited_scope` already makes.
+/// Every shape `exists_scope_name` cannot name is refused, this one included:
+/// leaving the bound unscoped binds and runs, silently answering from rows
+/// outside the bound. Losing the pushdown is the cheaper failure, and it is the
+/// trade `derive_row_limited_scope` already makes.
 #[test]
 fn test_unparse_semi_join_build_side_fetch_refuses_qualified_full_column_dialect()
 -> Result<()> {
@@ -3305,6 +3304,60 @@ fn test_unparse_left_semi_join_scopes_bounded_set_operation_build_side() -> Resu
     Ok(())
 }
 
+/// Builds `LeftSemi Join: t1.c = t2.c AND t1.d = t3.d` over a `t2 INNER JOIN t3`
+/// build side, optionally bounded, so the bounded and unbounded cases below
+/// differ only in the bound.
+fn multi_relation_build_side_semi_join(fetch: Option<usize>) -> Result<LogicalPlan> {
+    let schema = exists_fetch_schema();
+    let probe = table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?;
+    let build = table_scan(Some("t2"), &schema, Some(vec![0]))?.join_on(
+        table_scan(Some("t3"), &schema, Some(vec![0, 1]))?.build()?,
+        datafusion_expr::JoinType::Inner,
+        vec![col("t2.c").eq(col("t3.c"))],
+    )?;
+    // Applied only when asked: `limit(0, None)` still inserts a `Limit` node,
+    // and the unbounded cases are about a build side that carries no bound.
+    let build = match fetch {
+        Some(fetch) => build.limit(0, Some(fetch))?,
+        None => build,
+    }
+    .build()?;
+
+    LogicalPlanBuilder::from(probe)
+        .project(vec![col("t1.d")])?
+        .join_on(
+            build,
+            datafusion_expr::JoinType::LeftSemi,
+            vec![col("t1.c").eq(col("t2.c")), col("t1.d").eq(col("t3.d"))],
+        )?
+        .build()
+}
+
+/// Builds `LeftSemi Join: t.c = t.c` where the build side is the same relation as
+/// the probe, optionally bounded — the self-join whose correlation carries only a
+/// qualifier the probe also answers to.
+fn probe_qualified_self_join(fetch: Option<usize>) -> Result<LogicalPlan> {
+    let schema = exists_fetch_schema();
+    let probe = table_scan(Some("t"), &schema, Some(vec![0, 1]))?.build()?;
+    let build = table_scan_with_filter_and_fetch(
+        Some("t"),
+        &schema,
+        Some(vec![0]),
+        vec![],
+        fetch,
+    )?
+    .build()?;
+
+    LogicalPlanBuilder::from(probe)
+        .project(vec![col("t.d")])?
+        .join_on(
+            build,
+            datafusion_expr::JoinType::LeftSemi,
+            vec![col("t.c").eq(col("t.c"))],
+        )?
+        .build()
+}
+
 /// A correlation naming more than one of the build side's own inputs cannot be
 /// scoped: one derived table can answer to only one of those names, so no name
 /// keeps every reference bound to the relation it came from.
@@ -3321,25 +3374,7 @@ fn test_unparse_left_semi_join_scopes_bounded_set_operation_build_side() -> Resu
 /// refusal to become a scope.
 #[test]
 fn test_unparse_left_semi_join_refuses_multi_relation_build_side() -> Result<()> {
-    let schema = exists_fetch_schema();
-    let probe = table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?;
-    let build = table_scan(Some("t2"), &schema, Some(vec![0]))?
-        .join_on(
-            table_scan(Some("t3"), &schema, Some(vec![0, 1]))?.build()?,
-            datafusion_expr::JoinType::Inner,
-            vec![col("t2.c").eq(col("t3.c"))],
-        )?
-        .limit(0, Some(5))?
-        .build()?;
-
-    let plan = LogicalPlanBuilder::from(probe)
-        .project(vec![col("t1.d")])?
-        .join_on(
-            build,
-            datafusion_expr::JoinType::LeftSemi,
-            vec![col("t1.c").eq(col("t2.c")), col("t1.d").eq(col("t3.d"))],
-        )?
-        .build()?;
+    let plan = multi_relation_build_side_semi_join(Some(5))?;
 
     let unparser = Unparser::new(&UnparserPostgreSqlDialect {});
     let err = unparser
@@ -3369,25 +3404,7 @@ fn test_unparse_left_semi_join_refuses_multi_relation_build_side() -> Result<()>
 /// still emitted and still wrong.
 #[test]
 fn test_unparse_left_semi_join_refuses_probe_qualified_correlation() -> Result<()> {
-    let schema = exists_fetch_schema();
-    let probe = table_scan(Some("t"), &schema, Some(vec![0, 1]))?.build()?;
-    let build = table_scan_with_filter_and_fetch(
-        Some("t"),
-        &schema,
-        Some(vec![0]),
-        vec![],
-        Some(5),
-    )?
-    .build()?;
-
-    let plan = LogicalPlanBuilder::from(probe)
-        .project(vec![col("t.d")])?
-        .join_on(
-            build,
-            datafusion_expr::JoinType::LeftSemi,
-            vec![col("t.c").eq(col("t.c"))],
-        )?
-        .build()?;
+    let plan = probe_qualified_self_join(Some(5))?;
 
     let unparser = Unparser::new(&UnparserPostgreSqlDialect {});
     let err = unparser
@@ -3410,24 +3427,7 @@ fn test_unparse_left_semi_join_refuses_probe_qualified_correlation() -> Result<(
 #[test]
 fn test_unparse_left_semi_join_without_fetch_keeps_multi_relation_correlation()
 -> Result<()> {
-    let schema = exists_fetch_schema();
-    let probe = table_scan(Some("t1"), &schema, Some(vec![0, 1]))?.build()?;
-    let build = table_scan(Some("t2"), &schema, Some(vec![0]))?
-        .join_on(
-            table_scan(Some("t3"), &schema, Some(vec![0, 1]))?.build()?,
-            datafusion_expr::JoinType::Inner,
-            vec![col("t2.c").eq(col("t3.c"))],
-        )?
-        .build()?;
-
-    let plan = LogicalPlanBuilder::from(probe)
-        .project(vec![col("t1.d")])?
-        .join_on(
-            build,
-            datafusion_expr::JoinType::LeftSemi,
-            vec![col("t1.c").eq(col("t2.c")), col("t1.d").eq(col("t3.d"))],
-        )?
-        .build()?;
+    let plan = multi_relation_build_side_semi_join(None)?;
 
     let unparser = Unparser::new(&UnparserPostgreSqlDialect {});
     assert_snapshot!(
@@ -3437,30 +3437,16 @@ fn test_unparse_left_semi_join_without_fetch_keeps_multi_relation_correlation()
     Ok(())
 }
 
-/// The probe-qualified self-join, unbounded. Still emitted, and still wrong: the
-/// inner `FROM "t"` shadows the outer `"t"`, so the correlation is lost. That is
-/// the pre-existing defect spiceai/spiceai#12840 tracks, independent of any
-/// bound.
-///
-/// Recorded so the boundary of the refusal above is explicit — it covers the
-/// bounded build side, which is where a scope is demanded, and nothing else. Not
-/// an endorsement of this output; whoever lands the qualifier rewrite should
-/// expect this snapshot to change too.
+/// Pins that the refusal above is gated on the bound: unbounded, the same
+/// correlation is still emitted. This output is **known-incorrect** — the inner
+/// `FROM "t"` shadows the outer `"t"`, so the correlation is lost — and it is not
+/// an endorsement. That shadowing is the pre-existing defect independent of any
+/// bound, so whoever lands the qualifier rewrite should expect this snapshot to
+/// change too.
 #[test]
 fn test_unparse_left_semi_join_without_fetch_still_shadows_probe_qualifier() -> Result<()>
 {
-    let schema = exists_fetch_schema();
-    let probe = table_scan(Some("t"), &schema, Some(vec![0, 1]))?.build()?;
-    let build = table_scan(Some("t"), &schema, Some(vec![0]))?.build()?;
-
-    let plan = LogicalPlanBuilder::from(probe)
-        .project(vec![col("t.d")])?
-        .join_on(
-            build,
-            datafusion_expr::JoinType::LeftSemi,
-            vec![col("t.c").eq(col("t.c"))],
-        )?
-        .build()?;
+    let plan = probe_qualified_self_join(None)?;
 
     let unparser = Unparser::new(&UnparserPostgreSqlDialect {});
     assert_snapshot!(