Skip to content

Conversation

@maropu
Copy link
Member

@maropu maropu commented Jan 28, 2019

What changes were proposed in this pull request?

In master, GetMapValue nullable is always true;

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.

@maropu
Copy link
Member Author

maropu commented Jan 28, 2019

It seems ElementAt could have the same fix. This change is trivial, so I think it might be better to include anElementAt fix in this pr, too.

@SparkQA
Copy link

SparkQA commented Jan 28, 2019

Test build #101748 has finished for PR 23669 at commit ddd3b56.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Jan 28, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Jan 28, 2019

Test build #101756 has finished for PR 23669 at commit ddd3b56.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.
Merged to master.

jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants