-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-26637][SQL] Makes GetArrayItem nullability more precise #23566
Conversation
Test build #101316 has finished for PR 23566 at commit
|
ar(intOrdinal).nullable | ||
case GetArrayStructFields(CreateArray(ar), _, _, _, containsNull) | ||
if intOrdinal < ar.length => | ||
containsNull |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case GetArrayStructFields(CreateArray(elements), field, _, _, _) if intOrdinal < elements.length =>
elements(i).nullable || field.nullable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Test build #101475 has finished for PR 23566 at commit
|
1f9106f
to
40a598d
Compare
val f2 = StructField("b", IntegerType, nullable = true) | ||
val structType = StructType(f1 :: f2 :: Nil) | ||
val c = AttributeReference("c", structType, nullable = false)() | ||
val stArray1 = GetArrayStructFields(CreateArray(c :: Nil), f1, 0, 2, containsNull = f1.nullable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
containsNull
should be c.nullable
, according to the logic in ExtractValue.apply
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I'll update soon.
Test build #101503 has finished for PR 23566 at commit
|
|
||
val d = AttributeReference("d", structType, nullable = true)() | ||
val stArray3 = GetArrayStructFields(CreateArray(c :: d :: Nil), f1, 0, 2, | ||
containsNull = f1.nullable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Test build #101532 has finished for PR 23566 at commit
|
thanks, merging to master! |
## What changes were proposed in this pull request? In master, `GetMapValue` nullable is always true; https://github.com/apache/spark/blob/cf133e611020ed178f90358464a1b88cdd9b7889/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala#L371 But, If input key is foldable, we could make its nullability more precise. This fix is the same with SPARK-26637(#23566). ## How was this patch tested? Added tests in `ComplexTypeSuite`. Closes #23669 from maropu/SPARK-26747. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
## What changes were proposed in this pull request? In the master, GetArrayItem nullable is always true; https://github.com/apache/spark/blob/cf133e611020ed178f90358464a1b88cdd9b7889/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala#L236 But, If input array size is constant and ordinal is foldable, we could make GetArrayItem nullability more precise. This pr added code to make `GetArrayItem` nullability more precise. ## How was this patch tested? Added tests in `ComplexTypeSuite`. Closes apache#23566 from maropu/GetArrayItemNullability. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
## What changes were proposed in this pull request? In master, `GetMapValue` nullable is always true; https://github.com/apache/spark/blob/cf133e611020ed178f90358464a1b88cdd9b7889/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala#L371 But, If input key is foldable, we could make its nullability more precise. This fix is the same with SPARK-26637(apache#23566). ## How was this patch tested? Added tests in `ComplexTypeSuite`. Closes apache#23669 from maropu/SPARK-26747. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
… cases ## What changes were proposed in this pull request? In master, `ElementAt` nullable is always true; https://github.com/apache/spark/blob/be1cadf16dc70e22eae144b3dfce9e269ef95acc/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L1977 But, If input is an array and foldable, we could make its nullability more precise. This fix is based on SPARK-26637(#23566). ## How was this patch tested? Added tests in `CollectionExpressionsSuite`. Closes #23867 from maropu/SPARK-26965. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
What changes were proposed in this pull request?
In the master, GetArrayItem nullable is always true;
spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
Line 236 in cf133e6
But, If input array size is constant and ordinal is foldable, we could make GetArrayItem nullability more precise. This pr added code to make
GetArrayItem
nullability more precise.How was this patch tested?
Added tests in
ComplexTypeSuite
.