HyukjinKwon commented on code in PR #43352:
URL: https://github.com/apache/spark/pull/43352#discussion_r1427431997
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala:
##########
@@ -115,6 +115,17 @@ private[connect] object ErrorUtils extends Logging {
if (sparkThrowable.getErrorClass != null) {
sparkThrowableBuilder.setErrorClass(sparkThrowable.getErrorClass)
}
+ for (queryCtx <- sparkThrowable.getQueryContext) {
+ sparkThrowableBuilder.addQueryContexts(
+ FetchErrorDetailsResponse.QueryContext
+ .newBuilder()
+ .setObjectType(queryCtx.objectType())
+ .setObjectName(queryCtx.objectName())
+ .setStartIndex(queryCtx.startIndex())
Review Comment:
Alright, there are two issues. **TL;DR:**
1. There are many places that does not provide `Origin` that contains
`QueryContext` at `QueryCompilationErrors`. So the `QueryContext` is often
missing (e.g., the case above). cc @MaxGekk
2. . `Origin.context` is being directly used on Executor side. e.g.,
`origin.context` at `DivideYMInterval`. This seems wrongly returning
`SQLQueryContext` for DataFrame operations (from Executor side) in Spark
Connect server because we do not call `withOrigin` there. That's why this
specific code did not throw an exception from
https://github.com/apache/spark/pull/43334/files#diff-b3bc05fec45cd951053b2876c71c7730b63789cb4336a7537a6654c724db3241R586-R589
because it has never been `DataFrameContext` cc @heyihong @juliuszsompolski
5. The current logic in `ErrorUtils.scala` should handle the case of
`DataFrameQueryContext` e.g., `DataFrameQueryContext.stopIndex()` will throw an
exception. Should we:
- Set a default value in `DataFrameQueryContext.stopIndex()` instead of
throwing an exception? @MaxGekk
- Or, make the protobuf message for this optional, and throw an
exception from Spark Connect client sides? @heyihong @juliuszsompolski
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
