andygrove commented on code in PR #1912:
URL:
https://github.com/apache/datafusion-ballista/pull/1912#discussion_r3571120843
##########
ballista/client/tests/context_checks.rs:
##########
@@ -77,6 +77,40 @@ mod supported {
Ok(())
}
+ // An uncorrelated scalar subquery is executed first and its value inlined
+ // into the outer plan (rather than decorrelated to a join), so the outer
+ // query distributes without a `ScalarSubqueryExec`. See #1910.
+ #[rstest]
+ #[case::standalone(standalone_context())]
+ #[case::remote(remote_context())]
+ #[tokio::test]
+ async fn should_execute_uncorrelated_scalar_subquery(
+ #[future(awt)]
+ #[case]
+ ctx: SessionContext,
+ test_data: String,
+ ) -> datafusion::error::Result<()> {
+ ctx.register_parquet(
+ "test",
+ &format!("{test_data}/alltypes_plain.parquet"),
+ Default::default(),
+ )
+ .await?;
+
Review Comment:
Done — the test now runs `SET
datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery = true`
before the query, so it states that it covers the case where the optimizer is
free to plan a physical scalar subquery.
##########
ballista/client/tests/context_checks.rs:
##########
@@ -77,6 +77,40 @@ mod supported {
Ok(())
}
+ // An uncorrelated scalar subquery is executed first and its value inlined
+ // into the outer plan (rather than decorrelated to a join), so the outer
+ // query distributes without a `ScalarSubqueryExec`. See #1910.
+ #[rstest]
+ #[case::standalone(standalone_context())]
+ #[case::remote(remote_context())]
+ #[tokio::test]
+ async fn should_execute_uncorrelated_scalar_subquery(
+ #[future(awt)]
+ #[case]
+ ctx: SessionContext,
+ test_data: String,
+ ) -> datafusion::error::Result<()> {
+ ctx.register_parquet(
+ "test",
+ &format!("{test_data}/alltypes_plain.parquet"),
+ Default::default(),
+ )
+ .await?;
+
+ // Exactly the row holding the maximum id matches the inlined subquery
+ // value, so the count is 1.
+ let result = ctx
+ .sql("select count(*) as cnt from test where id = (select max(id)
from test)")
+ .await?
+ .collect()
+ .await?;
+ let expected = ["+-----+", "| cnt |", "+-----+", "| 1 |", "+-----+"];
+
+ assert_batches_eq!(expected, &result);
+
+ Ok(())
+ }
+
Review Comment:
Added `standalone_aqe` / `remote_aqe` cases (new
`standalone_context_with_aqe()` / `remote_context_with_aqe()` fixtures set
`ballista.planner.adaptive.enabled=true`). All four cases pass —
materialization happens in the client query planner before the plan reaches the
scheduler, so it is independent of which distributed planner runs. No issue
needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
