Skip to content

Commit bdd2796

Browse files
rdbluecloud-fan
authored andcommitted
[SPARK-24251][SQL] Add analysis tests for AppendData.
## What changes were proposed in this pull request? This is a follow-up to #21305 that adds a test suite for AppendData analysis. This also fixes the following problems uncovered by these tests: * Incorrect order of data types passed to `canWrite` is fixed * The field check calls `canWrite` first to ensure all errors are found * `AppendData#resolved` must check resolution of the query's attributes * Column names are quoted to show empty names ## How was this patch tested? This PR adds a test suite for AppendData analysis. Closes #22043 from rdblue/SPARK-24251-add-append-data-analysis-tests. Authored-by: Ryan Blue <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent 6c7bb57 commit bdd2796

File tree

3 files changed

+397
-13
lines changed

3 files changed

+397
-13
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2258,8 +2258,8 @@ class Analyzer(
22582258
if (expected.size < query.output.size) {
22592259
throw new AnalysisException(
22602260
s"""Cannot write to '$tableName', too many data columns:
2261-
|Table columns: ${expected.map(_.name).mkString(", ")}
2262-
|Data columns: ${query.output.map(_.name).mkString(", ")}""".stripMargin)
2261+
|Table columns: ${expected.map(c => s"'${c.name}'").mkString(", ")}
2262+
|Data columns: ${query.output.map(c => s"'${c.name}'").mkString(", ")}""".stripMargin)
22632263
}
22642264

22652265
val errors = new mutable.ArrayBuffer[String]()
@@ -2278,8 +2278,9 @@ class Analyzer(
22782278
if (expected.size > query.output.size) {
22792279
throw new AnalysisException(
22802280
s"""Cannot write to '$tableName', not enough data columns:
2281-
|Table columns: ${expected.map(_.name).mkString(", ")}
2282-
|Data columns: ${query.output.map(_.name).mkString(", ")}""".stripMargin)
2281+
|Table columns: ${expected.map(c => s"'${c.name}'").mkString(", ")}
2282+
|Data columns: ${query.output.map(c => s"'${c.name}'").mkString(", ")}"""
2283+
.stripMargin)
22832284
}
22842285

22852286
query.output.zip(expected).flatMap {
@@ -2301,12 +2302,15 @@ class Analyzer(
23012302
queryExpr: NamedExpression,
23022303
addError: String => Unit): Option[NamedExpression] = {
23032304

2305+
// run the type check first to ensure type errors are present
2306+
val canWrite = DataType.canWrite(
2307+
queryExpr.dataType, tableAttr.dataType, resolver, tableAttr.name, addError)
2308+
23042309
if (queryExpr.nullable && !tableAttr.nullable) {
23052310
addError(s"Cannot write nullable values to non-null column '${tableAttr.name}'")
23062311
None
23072312

2308-
} else if (!DataType.canWrite(
2309-
tableAttr.dataType, queryExpr.dataType, resolver, tableAttr.name, addError)) {
2313+
} else if (!canWrite) {
23102314
None
23112315

23122316
} else {

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -363,13 +363,14 @@ case class AppendData(
363363
override def output: Seq[Attribute] = Seq.empty
364364

365365
override lazy val resolved: Boolean = {
366-
query.output.size == table.output.size && query.output.zip(table.output).forall {
367-
case (inAttr, outAttr) =>
368-
// names and types must match, nullability must be compatible
369-
inAttr.name == outAttr.name &&
370-
DataType.equalsIgnoreCompatibleNullability(outAttr.dataType, inAttr.dataType) &&
371-
(outAttr.nullable || !inAttr.nullable)
372-
}
366+
table.resolved && query.resolved && query.output.size == table.output.size &&
367+
query.output.zip(table.output).forall {
368+
case (inAttr, outAttr) =>
369+
// names and types must match, nullability must be compatible
370+
inAttr.name == outAttr.name &&
371+
DataType.equalsIgnoreCompatibleNullability(outAttr.dataType, inAttr.dataType) &&
372+
(outAttr.nullable || !inAttr.nullable)
373+
}
373374
}
374375
}
375376

0 commit comments

Comments
 (0)