-
Notifications
You must be signed in to change notification settings - Fork 28.7k
[SPARK-52875][SQL] Simplify V2 expression translation if the input is context-independent-foldable #51569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -83,11 +84,20 @@ class V2ExpressionBuilder(e: Expression, isPredicate: Boolean = false) extends L | |||
case _ => false | |||
} | |||
|
|||
private def generateExpression( | |||
expr: Expression, isPredicate: Boolean = false): Option[V2Expression] = expr match { | |||
private def translateLiteral(l: Literal): Option[V2Expression] = l match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can return V2Expression
directly as the input is Literal
now.
@cloud-fan thanks for the review. Merging to master |
) | ||
} | ||
|
||
test("Partial constant folding of math functions") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The newly added test will fail in non-anis mode:
We can reproduce it locally:
SPARK_ANSI_SQL_MODE=false build/sbt "sql/testOnly org.apache.spark.sql.execution.datasources.v2.DataSourceV2StrategySuite"
[info] - Partial constant folding of math functions *** FAILED *** (4 milliseconds)
[info] can't convert to V2 expression: (LOG10(100.0) + cint#28) (DataSourceV2StrategySuite.scala:843)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1564)
[info] at org.scalatest.Assertions.fail(Assertions.scala:933)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:929)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1564)
[info] at org.apache.spark.sql.execution.datasources.v2.DataSourceV2StrategySuite.$anonfun$checkV2Conversion$1(DataSourceV2StrategySuite.scala:843)
[info] at scala.Option.getOrElse(Option.scala:201)
[info] at org.apache.spark.sql.execution.datasources.v2.DataSourceV2StrategySuite.checkV2Conversion(DataSourceV2StrategySuite.scala:843)
[info] at org.apache.spark.sql.execution.datasources.v2.DataSourceV2StrategySuite.$anonfun$new$13(DataSourceV2StrategySuite.scala:775)
@gengliangwang Do you have time to take a look? Thanks ~
What changes were proposed in this pull request?
If the input to V2 expression translation is context-independent and foldable (see PR #51282), we perform constant folding on the input and use the evaluated result.
For example:
1 + 1
becomes 2a < log2(8)
becomes a < 3.0Why are the changes needed?
This change broadens the coverage of V2 expression translation.
Does this PR introduce any user-facing change?
No
How was this patch tested?
New UT
Was this patch authored or co-authored using generative AI tooling?
No