FailedChanges

Summary

  1. [SPARK-25388][TEST][SQL] Detect incorrect nullable of DataType in the (details)
Commit c9d7d83ed5790aa272e969af36fd0cb90231111f by wenchen
[SPARK-25388][TEST][SQL] Detect incorrect nullable of DataType in the
result
## What changes were proposed in this pull request?
This PR can correctly cause assertion failure when incorrect nullable of
DataType in the result is generated by a target function to be tested.
Let us think the following example. In the future, a developer would
write incorrect code that returns unexpected result. We have to
correctly cause fail in this test since `valueContainsNull=false` while
`expr` includes `null`. However, without this PR, this test passes. This
PR can correctly cause fail.
``` test("test TARGETFUNCTON") {
val expr = TARGETMAPFUNCTON()
// expr = UnsafeMap(3 -> 6, 7 -> null)
// expr.dataType = (IntegerType, IntegerType, false)
  expected = Map(3 -> 6, 7 -> null)
checkEvaluation(expr, expected)
```
In
[`checkEvaluationWithUnsafeProjection`](https://github.com/apache/spark/blob/master/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala#L208-L235),
the results are compared using `UnsafeRow`. When the given `expected` is
[converted](https://github.com/apache/spark/blob/master/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala#L226-L227))
to `UnsafeRow` using the `DataType` of `expr`.
``` val expectedRow = UnsafeProjection.create(Array(expression.dataType,
expression.dataType)).apply(lit)
```
In summary, `expr` is
`[0,1800000038,5000000038,18,2,0,700000003,2,0,6,18,2,0,700000003,2,0,6]`
with and w/o this PR. `expected` is converted to
* w/o  this PR,
`[0,1800000038,5000000038,18,2,0,700000003,2,0,6,18,2,0,700000003,2,0,6]`
* with this PR,
`[0,1800000038,5000000038,18,2,0,700000003,2,2,6,18,2,0,700000003,2,2,6]`
As a result, w/o this PR, the test unexpectedly passes.
This is because, w/o this PR, based on given `dataType`, generated code
of projection for `expected` avoids to set nullbit.
```
                   // tmpInput_2 is expected
/* 155 */           for (int index_1 = 0; index_1 < numElements_1;
index_1++) {
/* 156 */             mutableStateArray_1[1].write(index_1,
tmpInput_2.getInt(index_1));
/* 157 */           }
```
With this PR, generated code of projection for `expected` always checks
whether nullbit should be set by `isNullAt`
```
                   // tmpInput_2 is expected
/* 161 */           for (int index_1 = 0; index_1 < numElements_1;
index_1++) {
/* 162 */
/* 163 */             if (tmpInput_2.isNullAt(index_1)) {
/* 164 */               mutableStateArray_1[1].setNull4Bytes(index_1);
/* 165 */             } else {
/* 166 */               mutableStateArray_1[1].write(index_1,
tmpInput_2.getInt(index_1));
/* 167 */             }
/* 168 */
/* 169 */           }
```
## How was this patch tested?
Existing UTs
Closes #22375 from kiszk/SPARK-25388.
Authored-by: Kazuaki Ishizaki <ishizaki@jp.ibm.com> Signed-off-by:
Wenchen Fan <wenchen@databricks.com>
The file was modifiedsql/hive/src/test/scala/org/apache/spark/sql/hive/execution/ObjectHashAggregateSuite.scala (diff)
The file was modifiedsql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelperSuite.scala (diff)
The file was modifiedsql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala (diff)
The file was modifiedsql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala (diff)