[SPARK-40482][SQL] Revert SPARK-24544 Print actual failure cause when look up function failed
#37896
Conversation
There was a problem hiding this comment.
not quite related but it's obviously redundant to try-catch and re-throw the error. externalCatalog.getFunction can throw NoSuchFunctionExcetion and we should just propagate it.
|
also cc @viirya |
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala
Outdated
Show resolved
Hide resolved
| TableFunctionRegistry.builtin.functionExists(name) | ||
| } | ||
|
|
||
| protected[sql] def failFunctionLookup( |
There was a problem hiding this comment.
Hmm, where do we set cause? As I see, we always call failFunctionLookup without cause (except for the test case).
There was a problem hiding this comment.
We used to pass the cause parameter before, when looking up builtin functions from Hive. The code path is gone now.
viirya
left a comment
viirya
left a comment
There was a problem hiding this comment.
Looks good. The cause seems no real usage now.
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
@cloud-fan . We need a new JIRA in this case to have a traceability. Could you file a JIRA issue for this reverting?
|
Oh, that's right. SPARK-24544 is long time ago, it is better to have a new JIRA. |
|
Let me create a JIRA for this. |
dongjoon-hyun
changed the title
[SPARK-24544][SQL] Print actual failure cause when look up function failed
dongjoon-hyun
changed the title
[SPARK-24544][SQL] Print actual failure cause when look up function failedSPARK-24544 Print actual failure cause when look up function failed
…ysis/NoSuchItemException.scala Co-authored-by: Liang-Chi Hsieh <[email protected]>
|
@dongjoon-hyun thanks for creating the JIRA ticket! |
|
The last commit is just to remove |
…n look up function failed` ### What changes were proposed in this pull request? This reverts apache#21790 because it's no longer needed. It kept the original error from Hive when Spark loads builtin functions from Hive, which no longer happens as Spark has implemented all builtin functions natively. ### Why are the changes needed? code cleanup ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests Closes apache#37896 from cloud-fan/error. Lead-authored-by: Wenchen Fan <[email protected]> Co-authored-by: Wenchen Fan <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
3 participants
What changes were proposed in this pull request?
This reverts #21790 because it's no longer needed. It kept the original error from Hive when Spark loads builtin functions from Hive, which no longer happens as Spark has implemented all builtin functions natively.
Why are the changes needed?
code cleanup
Does this PR introduce any user-facing change?
No
How was this patch tested?
existing tests