cloud-fan commented on code in PR #58130:
URL: https://github.com/apache/spark/pull/58130#discussion_r3841293091
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:
##########
@@ -2706,11 +2706,11 @@ case class Substring(str: Expression, pos: Expression,
len: Expression)
case class Right(str: Expression, len: Expression) extends RuntimeReplaceable
with ImplicitCastInputTypes with BinaryLike[Expression] {
- // Type the literal branches after R1: Substring returns plain STRING for a
CHAR(n)/VARCHAR(n)
- // input, so deriving the literals from str.dataType would leave the If with
branches of
- // different types. CheckAnalysis does not see inside a RuntimeReplaceable's
replacement, so
- // that mismatch would surface later as COMPLEX_EXPRESSION_UNSUPPORTED_INPUT.
- private lazy val resultType: DataType =
StringHelper.transformingStringResultType(str.dataType)
+ // Type the literal branches after ImplicitTypeCasts promotes CHAR/VARCHAR
to STRING.
+ // Substring then returns STRING, so the If branches match. CheckAnalysis
does not see inside
Review Comment:
**Non-blocking:**
Please remove the claimed CheckAnalysis/failure path here. Fixed-point
`CheckAnalysis` explicitly rejects an unresolved
`RuntimeReplaceable.replacement`, and the single-pass resolver does the same,
so a mismatched replacement would not later surface as
`COMPLEX_EXPRESSION_UNSUPPORTED_INPUT`. The preceding explanation about typing
`Right`'s `If` literals is sufficient.
##########
sql/core/src/test/scala/org/apache/spark/sql/CharVarcharTestSuite.scala:
##########
@@ -1794,14 +1848,14 @@ class BasicCharVarcharTestSuite extends
SharedSparkSession {
// silently lost: the HybridAnalyzer compares output schema and normalized
plan across the
Review Comment:
**Non-blocking:**
Please add a test-only assertion or hook that fails when any matrix query
does not enter dual-run. `HybridAnalyzer` also requires
`checkResolverGuard(plan)`; an ordinary unsupported reason makes `dualRun`
false and silently selects fixed-point analysis, while
`ANALYZER_SINGLE_PASS_RESOLVER_EXPOSE_RESOLVER_GUARD_FAILURE` only exposes
thrown guard failures. Without an execution-path assertion, this matrix can
pass without comparing analyzers.
--
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]
