rdtr commented on code in PR #13148:
URL: https://github.com/apache/gluten/pull/13148#discussion_r4174396426


##########
gluten-arrow/src/main/java/org/apache/gluten/vectorized/ColumnarBatchOutIterator.java:
##########
@@ -153,6 +153,12 @@ protected RuntimeException translateException(Exception e) 
{
         return schemaEx;
       }
     }
+    // A GlutenException raised by native/nested code is already translated; 
return it
+    // as-is rather than nesting it in another GlutenException. The 
schema-conversion
+    // special case above still runs first, so a schema error keeps its 
dedicated type.
+    if (e instanceof GlutenException) {

Review Comment:
   LGTM.
   
   One note for when #13135 lands: its `NativeCastException` extends 
`GlutenException`, so the pass-through added here would return it unchanged. 
#13135 checks `NativeCastException` at the top of 
`ColumnarBatchOutIterator.translateException`, before this pass-through, so the 
two work together as they are.
   
   But if the order changes during a rebase, native cast errors would silently 
skip translation, and nothing would catch it since there's no test here. Maybe 
worth a short comment near the pass-through saying that more specific 
translations must come before it? cc @felipepessoto



-- 
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]

Reply via email to