[
https://issues.apache.org/jira/browse/SPARK-57736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18092117#comment-18092117
]
Max Gekk commented on SPARK-57736:
----------------------------------
On the suggestion to just add {{require(name != null)}} to {{StructField}}:
These null-name NPEs (SPARK-57725 / SPARK-57726 / SPARK-57736) share one root
cause -- {{StructField}} permits a null name, which then NPEs at various
unguarded dereferences ({{name.toLowerCase}}, {{name.hashCode}},
{{name.toString}}, {{name.equalsIgnoreCase}}, ...). A single guard at the
source is tempting, but it is not a clean drop-in:
* {{StructField}} itself deliberately constructs a null name: the Kryo no-arg
constructor is {{protected def this() = this(null, null)}}. A {{require(name !=
null)}} in the primary constructor would run on that path and break Kryo
deserialization.
* It would reject schemas that are accepted today -- a behavior change, hence
master-only and not backportable. SPARK-57725 is already fixed and backported
to 4.2/4.1/4.0, and those maintenance lines cannot take a behavior-changing
require.
So the per-site null-safety fixes (tolerate/skip null names) are the pragmatic,
backportable path. If we want to stop the whack-a-mole on master, the better
place for a fail-fast guard is the user-facing schema boundary (e.g. validating
field names when a user schema enters via {{createDataFrame}}), not the
{{StructField}} constructor -- and that would be a separate, master-only change
that does not remove the need for the per-site fixes on maintenance branches.
> Fix NPE in CreateNamedStruct.dataType when a struct field name is null
> ----------------------------------------------------------------------
>
> Key: SPARK-57736
> URL: https://issues.apache.org/jira/browse/SPARK-57736
> Project: Spark
> Issue Type: Bug
> Components: SQL
> Affects Versions: 4.2.0, 4.1.2, 4.0.3, 4.3.0
> Reporter: Max Gekk
> Priority: Major
>
> h2. Summary
> {{CreateNamedStruct.dataType}} builds each field with
> {{StructField(name.toString, ...)}}.
> When a field name is {{null}}, {{name.toString}} throws a
> {{NullPointerException}}. This is
> reached eagerly while building a {{RowEncoder}} serializer, so it crashes
> before any
> analysis/resolution runs.
> h2. Affected code
> {{org.apache.spark.sql.catalyst.expressions.CreateNamedStruct.dataType}} in
> {{sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala}}:
> {code:scala}
> override lazy val dataType: StructType = {
> val fields = names.zip(valExprs).map {
> case (name, expr) =>
> val metadata = expr match {
> case ne: NamedExpression => ne.metadata
> case gsf: GetStructField => gsf.metadata
> case _ => Metadata.empty
> }
> StructField(name.toString, expr.dataType, expr.nullable, metadata) //
> NPE if name == null
> }
> StructType(fields)
> }
> {code}
> h2. Reproduction
> {code:scala}
> import org.apache.spark.sql.catalyst.expressions.{CreateNamedStruct, Literal}
> import org.apache.spark.sql.types.{IntegerType, StringType}
> CreateNamedStruct(Seq(Literal.create(null, StringType), Literal(1))).dataType
> {code}
> It also surfaces end-to-end via the encoder:
> {code:scala}
> import org.apache.spark.sql.Row
> import org.apache.spark.sql.types._
> val schema = StructType(Seq(StructField(null, IntegerType), StructField("b",
> IntegerType)))
> spark.createDataFrame(spark.sparkContext.parallelize(Seq(Row(1, 2))), schema)
> // builds RowEncoder -> NPE
> {code}
> Result:
> {code}
> java.lang.NullPointerException
> at
> org.apache.spark.sql.catalyst.expressions.CreateNamedStruct.$anonfun$dataType$9(complexTypeCreator.scala:473)
> at
> org.apache.spark.sql.catalyst.SerializerBuildHelper$.createSerializerForObject(...)
> at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder$.apply(...)
> {code}
> h2. Root cause
> A null field name is invalid -- {{CreateNamedStruct.checkInputDataTypes}}
> already rejects it
> ({{names.contains(null)}} -> {{UNEXPECTED_NULL}}) -- but {{dataType}}
> dereferences
> {{name.toString}} before type checking, and the encoder calls {{dataType}}
> directly. The
> underlying cause is that {{StructField}} permits a null name (no
> {{require(name != null)}}).
> h2. Proposed fix
> Make {{dataType}} null-safe and preserve the null name (consistent with
> SPARK-57725, which made
> {{AttributeSeq}} tolerate null-named attributes):
> {code:scala}
> StructField(if (name == null) null else name.toString, expr.dataType,
> expr.nullable, metadata)
> {code}
> A regression test in {{ComplexTypeSuite}} asserts {{dataType}} no longer
> throws and preserves the
> null field name.
> h2. Note
> This fixes the specific {{CreateNamedStruct.dataType}} NPE. The full
> {{createDataFrame(schemaWithNullFieldName)}} scenario hits additional,
> independent null-name sites
> further along (e.g. a {{StructField.name.equalsIgnoreCase}} schema comparison
> during resolution),
> which are separate pre-existing issues and out of scope here.
> h2. Related
> Found while backporting SPARK-57725 (NPE in {{AttributeSeq}} column
> resolution with a null-named
> attribute). Independent, pre-existing NPE on the same null-field-name edge
> case.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
