Skip to content
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-42236][SQL] Refine NULLABLE_ARRAY_OR_MAP_ELEMENT #39804

Closed
wants to merge 2 commits into from

Conversation

itholic
Copy link
Contributor

@itholic itholic commented Jan 30, 2023

What changes were proposed in this pull request?

This PR proposes to refine NULLABLE_ARRAY_OR_MAP_ELEMENT into main-sub classes structure.

NOT_NULL_CONSTRAINT_VIOLATION

  • ARRAY_ELEMENT
  • MAP_VALUE

Why are the changes needed?

The name of error class is misleading, and we can make this more generic so that we reuse for various situation.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Updated & added UTs.

@itholic
Copy link
Contributor Author

itholic commented Jan 30, 2023

We could also add NOT_NULL_CONSTRAINT_VIOLATION.STRUCT_FIELD for non-nullable StructField, but this case now it's buried under DATATYPE_MISMATCH.CAST_WITHOUT_SUGGESTION as below:

spark-sql> SELECT CAST(named_struct('a', null) AS STRUCT<a int not null>);
[DATATYPE_MISMATCH.CAST_WITHOUT_SUGGESTION] Cannot resolve "named_struct(a, NULL)" due to data type mismatch: cannot cast "STRUCT<a: VOID>" to "STRUCT<a: INT>".; line 1 pos 7;
'Project [unresolvedalias(cast(named_struct(a, null) as struct<a:int>), None)]

Let me migrate this case in separate ticket to facilitate review since the change would requires a non-trivial amount of code modifications.

@itholic
Copy link
Contributor Author

itholic commented Jan 30, 2023

cc @MaxGekk @srielau FYI

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

@itholic Could you fix, please:

org.scalatest.exceptions.DuplicateTestNameException: Duplicate test name: DATATYPE_MISMATCH.INVALID_JSON_SCHEMA: invalid top type passed to from_json()

@MaxGekk
Copy link
Member

MaxGekk commented Jan 31, 2023

+1, LGTM. Merging to master/3.4.
Thank you, @itholic.

@MaxGekk MaxGekk closed this in 1cba3b9 Jan 31, 2023
MaxGekk pushed a commit that referenced this pull request Jan 31, 2023
### What changes were proposed in this pull request?

This PR proposes to refine `NULLABLE_ARRAY_OR_MAP_ELEMENT` into main-sub classes structure.

`NOT_NULL_CONSTRAINT_VIOLATION`
- `ARRAY_ELEMENT`
- `MAP_VALUE`

### Why are the changes needed?

The name of error class is misleading, and we can make this more generic so that we reuse for various situation.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Updated & added UTs.

Closes #39804 from itholic/NULLABLE_ARRAY_OR_MAP_ELEMENT.

Authored-by: itholic <haejoon.lee@databricks.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 1cba3b9)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@itholic itholic deleted the NULLABLE_ARRAY_OR_MAP_ELEMENT branch April 22, 2023 05:47
snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
### What changes were proposed in this pull request?

This PR proposes to refine `NULLABLE_ARRAY_OR_MAP_ELEMENT` into main-sub classes structure.

`NOT_NULL_CONSTRAINT_VIOLATION`
- `ARRAY_ELEMENT`
- `MAP_VALUE`

### Why are the changes needed?

The name of error class is misleading, and we can make this more generic so that we reuse for various situation.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Updated & added UTs.

Closes apache#39804 from itholic/NULLABLE_ARRAY_OR_MAP_ELEMENT.

Authored-by: itholic <haejoon.lee@databricks.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 1cba3b9)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants