eugenegujing opened a new pull request, #7929:
URL: https://github.com/apache/texera/pull/7929

   ### What changes were proposed in this PR?
   
   The Projection operator's drop mode (`isDrop = true`) decides "which 
attributes to drop" in two places, and the two places used different rules for 
comparing attribute names. At compile time, the descriptor derives the output 
schema through `Schema.remove`, which ignores case, just like every other 
lookup in the `Schema` class. At runtime, the executor computed the kept 
attributes with `List.diff`, which matches names exactly.
   
   So the operator contradicted itself whenever a drop entry differed from the 
schema attribute only in case. Dropping `Field1` against a schema containing 
`field1`: the declared output schema removes the attribute, but the executor 
keeps it. A misspelled name is caught at plan time (`Schema.remove` rejects 
non-existent attributes), but a name that differs only in case passes that 
check and diverges silently.
   
   Users never see the divergence, because the engine rebuilds every output 
tuple against the declared schema by attribute name (`DataProcessor` calling 
`MapTupleLike.enforceSchema`), which discards the extra field — the final 
output is correct today, but by coincidence rather than by design. This PR 
makes the executor compute the same answer the descriptor declares, so 
correctness no longer depends on that coincidence.
   
   The change: the executor lowercases the drop list into a set and keeps an 
attribute unless its lowercased name is in that set — the same case-insensitive 
rule `Schema` uses everywhere. Kept attributes keep their original spelling and 
order; unknown names are still silently ignored; duplicate drop entries are 
still tolerated. The spec that pinned the old case-sensitive behavior is 
flipped to assert the unified semantics, and the stale cross-reference comments 
in `ProjectionOpDescSpec`/`ProjectionOpExecSpec` are updated.
   
   ### Any related issues, documentation, discussions?
   
   Fixes #7925.
   
   ### How was this PR tested?
   
   Existing Projection specs cover the change. The drop-mode case-matching spec 
in `ProjectionOpExecSpec` previously asserted that dropping `FIELD2` leaves 
`field2` in the output; it is renamed to "match drop names case-insensitively" 
and now asserts `field2` is removed, guarding against regression. Ran `sbt 
"WorkflowOperator/testOnly org.apache.texera.amber.operator.projection.*"` 
locally: 3 suites, 37 tests, all passed (0 failed, 0 canceled, 0 ignored), 
covering drop mode (case matching, unknown names, duplicates, ordering) and the 
unchanged keep mode.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Co-authored by: Claude Code (Claude Fable 5)


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

Reply via email to