Re: [PR] [MINOR][CORE] Do not re-wrap GlutenException in ClosableIterator.translateException [gluten]

["rdtr (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org