[SPARK-47263][SQL] Assign names to the legacy conditions _LEGACY_ERROR_TEMP_13[44-46]#46320
[SPARK-47263][SQL] Assign names to the legacy conditions _LEGACY_ERROR_TEMP_13[44-46]#46320PaysonXu wants to merge 1 commit intoapache:masterfrom
Conversation
|
@MaxGekk @cloud-fan would you please review this PR ? Actually, I'm not sure if rename 44 to internal err is appropriate. thx. |
There was a problem hiding this comment.
cc @dtenedor Is it really an internal error, please, confirm.
There was a problem hiding this comment.
@dtenedor kindly ping you. Please, have a look at the PR.
There was a problem hiding this comment.
@MaxGekk @dtenedor In the latest modification, i removed 44 and the reasons are as follows, thank you for reviewing:
- the only usage of _LEGACY_ERROR_TEMP_1344 is ResolveDefaultColumns#getExistenceDefaultValues when AnalysisException or MatchError matched when call analyze.
- AnalysisException thrown from analyze is already formatted(like INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION, INVALID_DEFAULT_VALUE.SUBQUERY_EXPRESSION) but was swallowed by _LEGACY_ERROR_TEMP_1344.
- MatchError can be considered a coding bug, so it is separated from AnalysisException as an internal error.
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
Outdated
Show resolved
Hide resolved
MaxGekk
left a comment
MaxGekk
left a comment
There was a problem hiding this comment.
Could you make PR's title shorter, and remove its tail from PR's description. Just name it as Assign names to the legacy conditions _LEGACY_ERROR_TEMP_13[44-46]
PaysonXu
changed the title
|
@PaysonXu Could you remove the text before |
PaysonXu
force-pushed
the
SPARK-47263
branch
4 times, most recently
from
a8a33da to
9ab66ae
Compare
sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@dtenedor kindly ping you. Please, have a look at the PR.
PaysonXu
force-pushed
the
SPARK-47263
branch
from
9ab66ae to
041e1e4
Compare
PaysonXu
force-pushed
the
SPARK-47263
branch
from
041e1e4 to
c9c0074
Compare
|
+1, LGTM. Merging to master. |
|
@PaysonXu Congratulations with your first contribution to Apache Spark! |
Thank you for your patient guidance. |
2 participants
What changes were proposed in this pull request?
rename err class _LEGACY_ERROR_TEMP_13[44-46]: 44 removed, 45 to DEFAULT_UNSUPPORTED, 46 to ADD_DEFAULT_UNSUPPORTED
Why are the changes needed?
replace legacy err class name with semantically explicits.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Re run the UT class modified in the PR (org.apache.spark.sql.sources.InsertSuite & org.apache.spark.sql.types.StructTypeSuite)
Was this patch authored or co-authored using generative AI tooling?
No