Copilot commented on code in PR #12709:
URL: https://github.com/apache/gluten/pull/12709#discussion_r3725495595
##########
openspec/changes/fix-outer-stack-marker-field0/tasks.md:
##########
@@ -0,0 +1,28 @@
+## 1. Reproduction & root-cause confirmation
+
+- [x] 1.1 Confirm the crash `Partition id (field 0) should be integer, but got
BOOLEAN` reproduces with `LATERAL VIEW OUTER stack(...)` + downstream hash
shuffle, and is 100% deterministic (data-independent).
+- [x] 1.2 Confirm via the code path that Velox `UnnestNode` appends a trailing
`BOOLEAN` marker for OUTER, and that `pullOutPostProject` has no `Stack` branch
(falls to `case _ => generate`).
+- [x] 1.3 Add regression test "test LATERAL VIEW OUTER stack followed by hash
shuffle" to `MiscOperatorSuite.scala` (asserts `GenerateExecTransformer`
present; compares against vanilla Spark).
+
+## 2. Implement the fix
+
+- [x] 2.1 In `PullOutGenerateProjectHelper.pullOutPostProject`, add `case _:
Stack if generate.outer =>` before `case _ => generate`.
+- [x] 2.2 In that branch: create boolean `isPresent` attribute;
`copy(generatorOutput = generate.generatorOutput :+ isPresent)`; wrap each
output column in `CaseWhen(Seq((isPresent, attr)), Literal(null,
attr.dataType))`; return `ProjectExec(generate.requiredChildOutput ++
newOutput, newGenerate)` (mirror the `Explode(_) if generate.outer` branch).
+- [x] 2.3 Verify no ordinality handling is needed for stack (Spark `LATERAL
VIEW OUTER stack` attaches no ordinality).
+
+## 3. Build & test
+
+- [x] 3.1 Compile `backends-velox`.
+- [x] 3.2 Run the new "test LATERAL VIEW OUTER stack followed by hash shuffle"
and confirm it passes (repackaged jar; the minimal repro `OUTER stack(2,...)
JOIN dim` returns the correct 4 rows instead of the `field 0 BOOLEAN` abort).
+- [x] 3.3 Run the existing inner-stack path (drop `OUTER`) and confirm no
regression (returns the same 4 join rows; inner stack takes `case _ =>
generate`, adds no column).
+- [x] 3.4 Repackage the Gluten jar for deployment (verified via the repackaged
jar on the minimal repro).
+
+## 4. Production validation
+
+- [x] 4.1 Run the reproducing production query and confirm the task no longer
aborts with `Partition id (field 0) should be integer, but got BOOLEAN` (event
log app_1782461172745_3126319: execId=8 original SQL16 and execId=9 ORDER BY
variant each ran 1477 tasks with 0 failures; the crash fingerprints `got
BOOLEAN`/`getFirstColumn`/`values_->capacity`/`INT_ARRAY vs BYTE_ARRAY` are
absent; both plans still contain GenerateExecTransformer + hashpartitioning
ColumnarExchange + SortMergeJoin, so the crash path is present and native, not
fallback-escaped).
Review Comment:
This task note includes a specific production event-log application ID and
execId/SQL identifiers. That kind of environment-specific identifier is usually
unnecessary in an open-source repo and can make the document harder to share.
Consider redacting the IDs while keeping the validation outcome and failure
signature.
##########
backends-velox/src/test/scala/org/apache/gluten/execution/MiscOperatorSuite.scala:
##########
@@ -1092,6 +1092,50 @@ class MiscOperatorSuite extends
VeloxWholeStageTransformerSuite with AdaptiveSpa
}
}
+ test("test LATERAL VIEW OUTER stack followed by hash shuffle") {
+ // Repro for: `Partition id (field 0) should be integer, but got BOOLEAN`
+ // (VeloxShuffleWriter.h getFirstColumn).
+ //
+ // With OUTER, Velox's Unnest appends a trailing BOOLEAN marker column.
The Stack path has
+ // no pullOutPostProject branch to consume that marker (unlike
explode/posexplode/inline),
+ // so the native output is one column wider than the declared schema and
every upstream
+ // column shifts by one. When the exploded key drives a hash-partition
shuffle, the int32
+ // hash_partition_key that must sit at field 0 is displaced by the boolean
marker and the
+ // columnar shuffle writer aborts.
Review Comment:
The comment explains the pre-fix behavior as if it is still true ("The Stack
path has no pullOutPostProject branch..."). After this change lands, that
wording becomes inaccurate and may confuse future readers of the regression
test. Rephrase it to describe the historical bug (or the failure mode when the
marker is not consumed).
--
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]
