sunchao commented on code in PR #56575:
URL: https://github.com/apache/spark/pull/56575#discussion_r3816855959
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:
##########
@@ -2690,29 +2690,40 @@ case class Substring(str: Expression, pos: Expression,
len: Expression)
since = "2.3.0",
group = "string_funcs")
// scalastyle:on line.size.limit
-case class Right(str: Expression, len: Expression) extends RuntimeReplaceable
- with ImplicitCastInputTypes with BinaryLike[Expression] {
-
- override lazy val replacement: Expression = If(
- IsNull(str),
- Literal(null, str.dataType),
- If(
- LessThanOrEqual(len, Literal(0)),
- Literal(UTF8String.EMPTY_UTF8, str.dataType),
- new Substring(str, UnaryMinus(len, failOnError = false))
- )
- )
+object Right extends DelegateFunction {
+ override val name: String = "right"
override def inputTypes: Seq[AbstractDataType] =
- Seq(
- StringTypeWithCollation(supportsTrimCollation = true),
- IntegerType
- )
- override def left: Expression = str
- override def right: Expression = len
- override protected def withNewChildrenInternal(
- newLeft: Expression, newRight: Expression): Expression = {
- copy(str = newLeft, len = newRight)
+ Seq(StringTypeWithCollation(supportsTrimCollation = true), IntegerType)
+
+ // At build time `str` is the not-yet-coerced argument (wrapped in an
`ImplicitCastInput` marker
+ // that delegates `dataType` to its child), so `str.dataType` is the *input*
type, which is not
+ // necessarily a string yet -- e.g. `right(12345, 2)` has an `IntegerType`
child the implicit cast
+ // will turn into a string. Use it for the null/empty branch literals only
when it is already a
+ // string-family type, so a CHAR(N)/VARCHAR(N) result (under
+ // `spark.sql.preserveCharVarcharTypeInfo`) or a non-default collation is
preserved through the
+ // `If` branch unification; otherwise fall back to plain `StringType`, the
type the implicit cast
+ // produces. Typing a UTF8String literal with a non-string type would be
invalid.
+ override def lower(args: Seq[Expression]): Expression = {
+ val str = args(0)
+ val len = args(1)
+ val litType = str.dataType match {
+ case _: StringType | _: CharType | _: VarcharType => str.dataType
+ case _ => StringType
+ }
+ // Keep both arguments single-use while the analyzer extracts window
expressions. The length
+ // is bound inside the non-null branch so right's null short-circuit is
preserved.
+ With(str) { case Seq(strRef) =>
Review Comment:
[P2] Follow-up on `0a33bc17`: the new constant-evaluation fix is still
incomplete under the fixed-point analyzer.
[`EvalHelper.prepareForEval`](https://github.com/apache/spark/blob/0a33bc177b56b174a63bb7f83a82f40e026ceae6/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EvalHelper.scala#L27-L39)
now unwraps delegates and `With`, but it leaves valid, resolved
`ImplicitCastInput`/`TypeCheckInput` markers in the result.
`ResolveInlineTables` and `ResolveIdentifierClause` run during Resolution,
before the separate `RemoveInputTypeMarkers` batch. Those markers are resolved
but still `Unevaluable` and nonfoldable.
Both of these reproduce on the current head with fixed-point analysis:
```sql
VALUES (right('abc', 1));
SELECT IDENTIFIER(right('xxa', 1)) FROM VALUES (1) AS t(a);
```
The first still raises
`INVALID_INLINE_TABLE.CANNOT_EVALUATE_EXPRESSION_IN_INLINE_TABLE`; the second
raises `NOT_A_CONSTANT_STRING.NOT_CONSTANT`. The newly added `right() supports
constant inline-table values` test also fails in the existing SQL suite. Could
preparation recurse into the children of **resolved** input markers, while
retaining failed markers for proper type-error reporting?
--
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]
