srielau commented on code in PR #58130:
URL: https://github.com/apache/spark/pull/58130#discussion_r3836546035
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -265,8 +265,12 @@ case class MultiGetJsonObject(
// scalastyle:on line.size.limit line.contains.tab
case class JsonTuple(children: Seq[Expression])
extends Generator
+ with ImplicitCastInputTypes
with QueryErrorsBase {
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq.fill(children.size)(StringTypeWithCollation(supportsTrimCollation =
true))
Review Comment:
Done. `JsonTuple` now mixes `ExpectsInputTypes` only (still keeps its
`NON_STRING_TYPE` check). INT and untyped NULL field names stay analysis
errors; CHAR/VARCHAR still promote to STRING on the existing
`ExpectsInputTypes` arm.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:
##########
@@ -4122,9 +4122,13 @@ case class Sentences(
*/
case class StringSplitSQL(
str: Expression,
- delimiter: Expression) extends BinaryExpression {
+ delimiter: Expression) extends BinaryExpression with
ImplicitCastInputTypes {
override def dataType: DataType =
- ArrayType(StringHelper.transformingStringResultType(str.dataType),
containsNull = false)
+ ArrayType(str.dataType, containsNull = false)
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq(
+ StringTypeWithCollation(supportsTrimCollation = true),
+ StringTypeWithCollation(supportsTrimCollation = true))
Review Comment:
Done. `StringSplitSQL` now mixes `ExpectsInputTypes` so a directly
constructed node still rejects non-string children. `SplitPart` keeps
`ImplicitCastInputTypes` for the SQL `split_part` path.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionHelper.scala:
##########
@@ -133,6 +139,71 @@ abstract class TypeCoercionHelper {
*/
def implicitCast(e: Expression, expectedType: AbstractDataType):
Option[Expression]
+ /**
+ * Where a plain string is expected, promote CHAR(n)/VARCHAR(n) to unbounded
STRING the same way
+ * SHORT promotes to INT, and return the promoted type.
+ *
+ * CharType and VarcharType extend StringType, so an expectation such as
+ * `StringTypeWithCollation` accepts them as-is and the implicit cast rules
leave the length
+ * constraint in place. Expressions that then require all their string
inputs to share a single
+ * type (`overlay`, `string_agg`, ...) cannot unify CHAR(n) with STRING, and
RuntimeReplaceable
+ * ones (`right`) build literals from the constrained type that no longer
match their other
+ * branches.
+ *
+ * The expectation must actually mention a string type (or an array of
strings). Promoting at an
+ * `AnyDataType` site would strip the length from pass-through expressions
such as `max`, `lag`,
+ * and `element_at`, which are required to preserve CHAR/VARCHAR.
+ */
+ protected def charVarcharToPlainString(
+ inType: DataType,
+ expectedType: AbstractDataType): Option[DataType] = {
+ if (!conf.charVarcharStandardSemantics) {
+ return None
+ }
+ inType match {
+ case st: StringType if !StringHelper.isPlainString(st) =>
+ val plain = StringHelper.plainStringType(st)
+ if (expectsStringType(expectedType) &&
expectedType.acceptsType(plain)) {
+ Some(plain)
+ } else {
+ None
+ }
+ case ArrayType(et, containsNull) =>
+ arrayElementExpectation(expectedType).flatMap { elemExpected =>
+ charVarcharToPlainString(et, elemExpected).map(ArrayType(_,
containsNull))
+ }
+ case _ => None
+ }
+ }
+
+ private def expectsStringType(expectedType: AbstractDataType): Boolean =
expectedType match {
+ case _: StringType => true
+ case _: AbstractStringType => true
+ case TypeCollection(types) => types.exists(expectsStringType)
+ case _ => false
+ }
+
+ private def arrayElementExpectation(expectedType: AbstractDataType):
Option[AbstractDataType] =
+ expectedType match {
+ case AbstractArrayType(elem) => Some(elem)
+ case ArrayType(elem, _) => Some(elem)
+ case TypeCollection(types) =>
types.view.flatMap(arrayElementExpectation).headOption
+ case _ => None
+ }
+
+ /**
+ * Concat/Elt stringify non-binary inputs. Promote CHAR/VARCHAR to unbounded
STRING with the
+ * same collation instead of targeting the default UTF8_BINARY StringType,
which would not
+ * accept a collated constrained type.
+ */
+ protected def implicitCastToString(e: Expression): Expression = e.dataType
match {
+ case st: StringType
+ if conf.charVarcharStandardSemantics &&
!StringHelper.isPlainString(st) =>
+ implicitCast(e, StringHelper.plainStringType(st)).getOrElse(e)
+ case _ =>
+ implicitCast(e, StringType).getOrElse(e)
+ }
+
Review Comment:
Done. `implicitCastToString` now calls `charVarcharToPlainString` with
`StringTypeWithCollation(supportsTrimCollation = true)` and only falls back to
`implicitCast(..., StringType)` for non-strings.
Added Concat/Elt coverage for `CHAR(n) COLLATE UTF8_LCASE` in
`TypeCoercionSuite` (legacy and ANSI rules), `BasicCharVarcharTestSuite`, and
`charvarchar-standard-semantics.sql`. The result type is `string collate
UTF8_LCASE`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercionHelper.scala:
##########
@@ -651,14 +718,20 @@ abstract class TypeCoercionHelper {
case e: ExpectsInputTypes if e.inputTypes.nonEmpty =>
// Convert NullType into some specific target type for
ExpectsInputTypes that don't do
- // general implicit casting.
+ // general implicit casting. Also promote CHAR/VARCHAR to STRING here:
these
+ // expressions skip ImplicitCastInputTypes, so without this the length
constraint would
+ // remain on the child.
val children: Seq[Expression] = e.children.zip(e.inputTypes).map {
case (in, expected) =>
- if (in.dataType == NullType && !expected.acceptsType(NullType)) {
- Literal.create(null, expected.defaultConcreteType)
- } else {
- in
- }
+ charVarcharToPlainString(in.dataType, expected)
Review Comment:
Agreed. JsonTuple and StringSplitSQL now use this `ExpectsInputTypes` path
rather than `ImplicitCastInputTypes`.
--
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]
