spark-test commented on code in PR #59882:
URL: https://github.com/apache/spark/pull/59882#discussion_r4406316199
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala:
##########
@@ -1640,6 +1641,31 @@ class StringExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
Literal.create(null, IntegerType), Literal.create(null, IntegerType)),
null)
}
+ for ((expression, expected) <- Seq(
+ (ToNumber(Literal("12"), Literal("99")), Decimal(12)),
+ (TryToNumber(Literal("12"), Literal("99")), Decimal(12)),
+ (ToCharacter(Literal(Decimal(12)), Literal("99")), "12"))) {
+ test(s"number format evaluator copies isolate parser state:
${expression.prettyName}") {
+ checkEvaluation(expression, expected)
+ val first = expression.freshCopyIfContainsStatefulExpression()
+ val second = expression.freshCopyIfContainsStatefulExpression()
+ assert(first ne expression)
+ assert(second ne first)
+ assert(first.foldable == expression.foldable)
Review Comment:
**Nit (P3):** Both sides of this comparison are always the same value.
`ToNumber`, `TryToNumber`, and `ToCharacter` don't override `foldable`, so each
side is `BinaryExpression.foldable` (`left.foldable && right.foldable`), and
`freshCopyIfContainsStatefulExpression()` rebuilds the node around the same
`Literal` children.
So a regression in either place that controls folding would slip through. If
`foldable` started consulting `stateful`, both sides would turn false together.
If `ConstantFolding` started skipping stateful expressions, both would stay
true while folding stopped. The `checkEvaluation` calls compare only evaluated
values, so they wouldn't notice an unfolded expression either.
The PR description lists preserved foldability among the tested properties,
but as written nothing in this suite fails if literal-only `to_number`,
`try_to_number`, or numeric `to_char` calls stop being constant-folded.
Production folding is correct at this head; the gap is only in what the test
can detect.
**Verification:**
- **Behavior:** For to_number, try_to_number, and numeric to_char over
literal inputs, the test checks that the original expression and both fresh
copies report foldable, and that optimizing a projection of the expression
yields a literal equal to the expected conversion result.
- **Inspection:** Each foldability check in the test compares against a
fixed expectation (the expression is foldable, and the optimized expression is
a literal) rather than against another value derived the same way, so the check
fails if these expressions stop being foldable or stop being constant-folded.
--
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]
