felipepessoto opened a new pull request, #12290:
URL: https://github.com/apache/gluten/pull/12290

   ## What changes are proposed in this pull request?
   
   `SubstraitVeloxExprConverter::toVeloxExpr(const 
::substrait::Expression::FieldReference&, const RowTypePtr&)` resolves a nested 
`struct_field` reference by walking the chain and descending one level at a 
time with `inputColumnType = asRowType(inputColumnType->childAt(idx))`.
   
   When the field path traverses a **non-struct child** — for example a field 
nested under an array — `asRowType()` returns `nullptr`, and the next loop 
iteration dereferenced that null `RowType`, crashing the whole (forked) JVM 
with a `SIGSEGV` in `libvelox.so`:
   
   ```
   # C  [libvelox.so+0x214951c]  
gluten::SubstraitVeloxExprConverter::toVeloxExpr(substrait::Expression_FieldReference
 const&, std::shared_ptr<facebook::velox::RowType const> const&)+0x9c
   ```
   
   Because a `SIGSEGV` is not a catchable C++ exception, 
`SubstraitToVeloxPlanValidator` (which wraps expression conversion in 
`try/catch (VeloxException)`) could not catch it and fall back — the process 
died outright.
   
   This PR replaces the unchecked navigation with `VELOX_USER_CHECK` guards 
that throw a `VeloxUserError`:
   
   - the field-reference index is within range (the raw 
`RowType::childAt`/`nameOf` use unchecked `operator[]`), and
   - the child being descended into is actually a struct/row.
   
   With these checks, an unsupported nested reference makes plan validation 
fail cleanly and the query falls back to vanilla Spark instead of crashing the 
JVM.
   
   ## How was this patch tested?
   
   Added `cpp/velox/tests/SubstraitVeloxExprConverterTest.cc`, a unit test that 
builds the exact crashing `FieldReference` — a reference descending into an 
array column — and asserts the converter now throws a `VeloxUserError` (via 
`VELOX_ASSERT_THROW`) instead of crashing the process; it also covers an 
out-of-range field index. Before this change the same input aborts the JVM with 
the `SIGSEGV` shown above.
   
   This is the same code path hit by Delta Lake's `UpdateSuite` test `"nested 
data support - analysis error - updating array type"` (an `UPDATE` whose field 
path descends through an array), which is exercised by the Gluten Delta Spark 
UT CI: before this change the forked test JVM aborts with the SIGSEGV; after 
it, validation falls back to vanilla Spark and the query is handled correctly.
   
   `clang-format-15` reports no changes.
   
   ## Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: GitHub Copilot CLI (claude-opus-4.8)
   


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