Skip to content

Conversation

@itholic
Copy link
Contributor

@itholic itholic commented Feb 12, 2023

What changes were proposed in this pull request?

This PR proposes to assign name to _LEGACY_ERROR_TEMP_2332, "UNSUPPORTED_DATASOURCE_FOR_DIRECT_QUERY".

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*

@github-actions github-actions bot added the SQL label Feb 12, 2023
@github-actions github-actions bot added the CORE label Feb 13, 2023
@itholic itholic changed the title [DO-NOT-MERGE][TEST] LEGACY_2332 [SPARK-42323][SQL] Assign name to _LEGACY_ERROR_TEMP_2332 Feb 13, 2023
@itholic itholic marked this pull request as ready for review February 13, 2023 05:34
@itholic
Copy link
Contributor Author

itholic commented Feb 13, 2023

cc @MaxGekk @srielau Could you review the error class when you find some time? 🙏

@MaxGekk
Copy link
Member

MaxGekk commented Feb 19, 2023

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

messageParameters = Map("msg" -> e.getMessage),
cause = e)
errorClass = "UNSUPPORTED_DATASOURCE_FOR_DIRECT_QUERY",
messageParameters = Map("dataSourceType" -> u.multipartIdentifier.head),
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not just assigning an error class name, but changing the error message as well. It's good to put the data source type in the error message, but I don't think we can simply ignore the original error message e.getMessage. @itholic can you fix it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix it before 3.5, as it's a regression

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, seems like the #42124 already fixes the bug from UNSUPPORTED_DATASOURCE_FOR_DIRECT_QUERY ??

Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure? I don't see it passing the original error message.

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.

4 participants