Skip to content

Conversation

@itholic
Copy link
Contributor

@itholic itholic commented Feb 5, 2023

What changes were proposed in this pull request?

This PR proposes to assign name to _LEGACY_ERROR_TEMP_2127, "DUPLICATED_MAP_KEY".

Why are the changes needed?

We should assign proper name to LEGACY_ERROR_TEMP*

Does this PR introduce any user-facing change?

No

How was this patch tested?

./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*

@itholic
Copy link
Contributor Author

itholic commented Feb 5, 2023

cc @srielau @MaxGekk @cloud-fan Please review this error class ticket when you find some time.

],
"sqlState" : "22012"
},
"DUPLICATED_MAP_KEY" : {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this associated with a function? Can we name it?

Copy link
Contributor Author

@itholic itholic Feb 6, 2023

Choose a reason for hiding this comment

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

I believe it seems not related to specific function, but can't 100% sure since I haven't enough context for Scala-side functions yet.
@MaxGekk Could you have an answer for this, if we name any specific function for this error class?

Copy link
Member

Choose a reason for hiding this comment

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

This error can occur in multiple functions:

  • map_concat
  • map_from_entries
  • map
  • map_from_arrays
  • str_to_map
  • transform_keys

Copy link
Contributor Author

@itholic itholic Feb 7, 2023

Choose a reason for hiding this comment

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

Yeah, IMHO maybe it is okay to leave it as is, because I believe "DUPLICATED_MAP_KEY" is pretty intuitive when users face this error during use such a map function?

…xecutionErrors.scala

Co-authored-by: Maxim Gekk <max.gekk@gmail.com>
@MaxGekk
Copy link
Member

MaxGekk commented Feb 8, 2023

+1, LGTM. Merging to master/3.4.
Thank you, @itholic and @srielau for review.

@MaxGekk MaxGekk closed this in 4b50a46 Feb 8, 2023
MaxGekk pushed a commit that referenced this pull request Feb 8, 2023
### What changes were proposed in this pull request?

This PR proposes to assign name to _LEGACY_ERROR_TEMP_2127, "DUPLICATED_MAP_KEY".

### Why are the changes needed?

We should assign proper name to _LEGACY_ERROR_TEMP_*

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

No

### How was this patch tested?

`./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*`

Closes #39890 from itholic/LEGACY_2127.

Lead-authored-by: itholic <haejoon.lee@databricks.com>
Co-authored-by: Haejoon Lee <44108233+itholic@users.noreply.github.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 4b50a46)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@itholic itholic deleted the LEGACY_2127 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 assign name to _LEGACY_ERROR_TEMP_2127, "DUPLICATED_MAP_KEY".

### Why are the changes needed?

We should assign proper name to _LEGACY_ERROR_TEMP_*

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

No

### How was this patch tested?

`./build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite*`

Closes apache#39890 from itholic/LEGACY_2127.

Lead-authored-by: itholic <haejoon.lee@databricks.com>
Co-authored-by: Haejoon Lee <44108233+itholic@users.noreply.github.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 4b50a46)
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

Development

Successfully merging this pull request may close these issues.

3 participants