sunchao commented on code in PR #7283:
URL: https://github.com/apache/datafusion-comet/pull/7283#discussion_r4405741498
##########
spark/src/test/scala/org/apache/comet/rules/CometExecRuleSuite.scala:
##########
@@ -151,6 +155,847 @@ class CometExecRuleSuite extends CometTestBase {
}.sum
}
+ test("make_date preserves row demand below lazy consumers, including reused
native plans") {
+ withSQLConf(
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false",
+ CometConf.COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED.key -> "true") {
+ val input = createSparkPlan(spark, "SELECT * FROM VALUES (2024), (2025)
AS t(y)")
+ val sample = createSparkPlan(
+ spark,
+ "SELECT * FROM (SELECT * FROM range(8)) TABLESAMPLE (50
PERCENT)").collectFirst {
+ case op: SampleExec => op
+ }.get
+ val consumers: Seq[(String, SparkPlan => SparkPlan)] = Seq(
+ ("LIMIT", child => CollectLimitExec(1, child)),
+ ("LIMIT", child => LocalLimitExec(1, child)),
+ ("LIMIT", child => GlobalLimitExec(1, child)),
+ ("SAMPLE", child => sample.withNewChildren(Seq(child))),
+ ("FILTER", child => FilterExec(GreaterThan(child.output.head,
Literal(0)), child)))
+ for (ansi <- Seq(false, true)) {
+ def project: SparkPlan = ProjectExec(
+ Seq(
+ input.output.head,
+ Alias(MakeDate(input.output.head, Literal(1), Literal(1), ansi),
"date")()),
+ input)
+ val native = applyCometExecRule(project)
+ assert(countOperators(native, classOf[CometProjectExec]) == 1,
native.treeString)
+ for ((boundary, consume) <- consumers; reused <- Seq(false, true)) {
+ val child = if (reused) applyCometExecRule(project) else project
+ val converted = applyCometExecRule(consume(child))
+ for (result <- Seq(converted, applyCometExecRule(converted))) {
+ withClue(s"ansi=$ansi reused=$reused
$boundary\n${result.treeString}") {
+ assert(countOperators(result, classOf[ProjectExec]) == 1)
+ assert(countOperators(result, classOf[CometProjectExec]) == 0)
+ assert(
+ new ExtendedExplainInfo()
+ .getFallbackReasons(result)
+ .exists(_.contains(s"make_date requires Spark evaluation
below $boundary")))
+ }
+ }
+ }
+
+ // These consumers evaluate their complete input independently of the
outer limit.
+ val materializers: Seq[SparkPlan => SparkPlan] = Seq(
+ child => SortExec(Seq(SortOrder(child.output.head, Ascending)),
false, child),
+ child => ShuffleExchangeExec(SinglePartition, child))
+ materializers.foreach { materialize =>
+ val result = applyCometExecRule(LocalLimitExec(1,
materialize(project)))
+ assert(countOperators(result, classOf[CometProjectExec]) == 1,
result.treeString)
+ assert(
+ !new ExtendedExplainInfo()
+ .getFallbackReasons(result)
+ .exists(_.contains("make_date requires Spark evaluation below
LIMIT")))
+ }
+ }
+ }
+ }
+
+ test("make_date shuffle keys remain eager below a limit") {
+ withSQLConf(
+ SQLConf.ANSI_ENABLED.key -> "true",
+ SQLConf.ADAPTIVE_EXECUTION_ENABLED.key -> "false",
+ CometConf.COMET_SHUFFLE_ENABLED.key -> "true",
+ CometConf.COMET_SHUFFLE_MODE.key -> "native",
+ CometConf.COMET_SHUFFLE_NATIVE_HASH_PARTITIONING_ENABLED.key -> "true") {
+ for (invalid <- Seq(false, true)) {
+ val rows = Seq.fill(20)((2024, 1, 1)) :+ (2024, if (invalid) 13 else
1, 1)
Review Comment:
[P2] Make the appended tuple explicit for Scala 2.12
The exact-head [Spark 3.5 strict-warnings CI
job](https://github.com/apache/datafusion-comet/actions/runs/40760678648/job/123078146951)
fails at this line: Scala 2.12 adapts the three arguments to a tuple and emits
the argument-adaptation warning, which the job treats as an error. This
prevents the Spark 3.5 test sources from compiling under the required strict
configuration. Wrap the tuple in another pair of parentheses (`:+ ((2024, if
(invalid) 13 else 1, 1))`) so it is passed as a single argument without
adaptation.
--
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]
