Skip to content

Conversation

@JinHelin404
Copy link
Contributor

What changes were proposed in this pull request?

Change the error class_LEGACY_ERROR_TEMP_2022 to internal error as it cannot be accessed by public API.

Why are the changes needed?

User experience improvement

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tests already exist.

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the SQL label Feb 28, 2024
@JinHelin404 JinHelin404 changed the title [SPARK-43255][SQL]Replace the error class _LEGACY_ERROR_TEMP_2020 by an internal error [WIP][SPARK-43255][SQL]Replace the error class _LEGACY_ERROR_TEMP_2020 by an internal error Feb 28, 2024
@JinHelin404 JinHelin404 changed the title [WIP][SPARK-43255][SQL]Replace the error class _LEGACY_ERROR_TEMP_2020 by an internal error [SPARK-43255][SQL]Replace the error class _LEGACY_ERROR_TEMP_2020 by an internal error Feb 28, 2024
@amaliujia
Copy link
Contributor

amaliujia commented Feb 28, 2024

If I am not wrong: StaticInvoke could trigger this error and StaticInvoke could be used by a user?

I saw in the past users use StaticInvoke in their library.

@amaliujia
Copy link
Contributor

cc @MaxGekk

@eejbyfeldt
Copy link
Contributor

@MaxGekk To me it seems correct to make it an internal error. I think the cases it could come up from using user defined types in the Scala API are meant to be captured earlier by other code and it failing with this would indicate a spark bug.

If I am not wrong: StaticInvoke could trigger this error and StaticInvoke could be used by a user?

I saw in the past users use StaticInvoke in their library.

My understanding is that classes org.apache.spark.sql.catalyst were consider semi private/developer api. But I have not really found the documentation where this is clarified so maybe I have misunderstood something. Anyway to me it does not seem realistic to provide good errors for all possible missuses of classes that live in there.

@MaxGekk MaxGekk changed the title [SPARK-43255][SQL]Replace the error class _LEGACY_ERROR_TEMP_2020 by an internal error [SPARK-43255][SQL] Replace the error class _LEGACY_ERROR_TEMP_2020 by an internal error Mar 1, 2024
@MaxGekk
Copy link
Member

MaxGekk commented Mar 1, 2024

@JinHelin404 Do you have an account at OSS JIRA: https://issues.apache.org/jira/browse/SPARK-43255? If so, please, leave a comment in the ticket (I will assign it to you) otherwise submit a request for the account.

@JinHelin404
Copy link
Contributor Author

@JinHelin404 Do you have an account at OSS JIRA: https://issues.apache.org/jira/browse/SPARK-43255? If so, please, leave a comment in the ticket (I will assign it to you) otherwise submit a request for the account.

@MaxGekk I have left a comment.

@MaxGekk
Copy link
Member

MaxGekk commented Mar 1, 2024

+1, LGTM. Merging to master.
Thank you, @JinHelin404 and @eejbyfeldt @amaliujia for review.

@MaxGekk MaxGekk closed this in a5a2822 Mar 1, 2024
TakawaAkirayo pushed a commit to TakawaAkirayo/spark that referenced this pull request Mar 4, 2024
… an internal error

### What changes were proposed in this pull request?

Change the error class_LEGACY_ERROR_TEMP_2022 to internal error as it cannot be accessed by public API.

### Why are the changes needed?

User experience improvement

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

No

### How was this patch tested?

Tests already exist.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#45302 from JinHelin404/SPARK-43255.

Authored-by: JineHelin404 <605188046@qq.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
ericm-db pushed a commit to ericm-db/spark that referenced this pull request Mar 5, 2024
… an internal error

### What changes were proposed in this pull request?

Change the error class_LEGACY_ERROR_TEMP_2022 to internal error as it cannot be accessed by public API.

### Why are the changes needed?

User experience improvement

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

No

### How was this patch tested?

Tests already exist.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes apache#45302 from JinHelin404/SPARK-43255.

Authored-by: JineHelin404 <605188046@qq.com>
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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants