[SPARK-51805] [SQL] Get function with improper argument should throw proper exception instead of an internal one #50590
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaxGekk
reviewed
Apr 15, 2025
Member
MaxGekk
left a comment
MaxGekk
left a comment
There was a problem hiding this comment.
Could you address another types of null:
spark-sql (default)> SELECT get(cast(null as string), 0);
[INTERNAL_ERROR] The Spark SQL phase analysis failed with an internal error. You hit a bug in Spark or the Spark plugins you use. Please, report this bug to the corresponding communities or vendors, and provide the full stack trace. SQLSTATE: XX000
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
Outdated
Show resolved
Hide resolved
sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
Outdated
Show resolved
Hide resolved
Contributor
Author
|
After offline discussion with @srielau we agreed that throwing is the way to go to make it on par with |
mihailoale-db
changed the title
mihailoale-db
changed the title
mihailoale-db
force-pushed
the
getnull
branch
4 times, most recently
from
5cc7a9d to
069d411
Compare
Contributor
Author
|
@MaxGekk PTAL when you have time. Thanks |
...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
Outdated
Show resolved
Hide resolved
MaxGekk
reviewed
Apr 23, 2025
...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
Outdated
Show resolved
Hide resolved
sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala
Outdated
Show resolved
Hide resolved
Contributor
Author
|
@MaxGekk PTAL when you have time. Thanks |
cloud-fan
reviewed
Apr 25, 2025
|
|
||
| override def checkInputDataTypes(): TypeCheckResult = { | ||
| (left.dataType, right.dataType) match { | ||
| case (_: ArrayType, e2) if !e2.isInstanceOf[IntegralType] => |
Contributor
There was a problem hiding this comment.
do we have a test to examine this branch?
Contributor
Author
There was a problem hiding this comment.
Yes, they have existed before (please check array.sql). But on the other hand select get(array(1),null) passes because GetArrayItem extends ExpectsInputTypes and because of that NullType is casted to IntegerType. Should we remove this check as it is noop?
Contributor
Author
There was a problem hiding this comment.
I would keep it for safety (so we don't throw internal error if we somehow fail to apply type coercion rule). @cloud-fan
cloud-fan
approved these changes
Apr 25, 2025
Contributor
|
thanks, merging to master! |
yhuang-db
pushed a commit
to yhuang-db/spark
that referenced
this pull request
Jun 9, 2025
…roper exception instead of an internal one ### What changes were proposed in this pull request? Following query throw `Cannot cast NullType to Arraytype`: ``` SELECT get(null, 0); ``` instead of throwing a more user friendly one. I propose that we fix that. ### Why are the changes needed? To correct behavior of `get` function. ### Does this PR introduce _any_ user-facing change? Query that were failing with internal error are now throwing a more user friendly one. ### How was this patch tested? Added tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#50590 from mihailoale-db/getnull. Authored-by: mihailoale-db <mihailo.aleksic@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Following query throw
Cannot cast NullType to Arraytype:instead of throwing a more user friendly one. I propose that we fix that.
Why are the changes needed?
To correct behavior of
getfunction.Does this PR introduce any user-facing change?
Query that were failing with internal error are now throwing a more user friendly one.
How was this patch tested?
Added tests.
Was this patch authored or co-authored using generative AI tooling?
No.