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

   ### What changes were proposed in this PR?
   
   Follow-up to #6661, addressing [this review 
comment](https://github.com/apache/texera/pull/6661#discussion_r3648708075): a 
loop-body operator that emits its own boundary state 
(`produce_state_on_start/finish` — a public API on both engine sides) sends it 
with the "no loop" envelope (counter `0`, `loop_start_id ""`). The LoopEnd 
matching branch treated **every** counter-0 frame as the loop's own boundary 
state:
   
   ```
   LoopStart ──(0, LS-id)──▶ stateful body op ──▶ LoopEnd
                                │ produce_state_on_finish
                                └──(0, "")────────▶ LoopEnd   ← arrives AFTER 
the loop state
   ```
   
   Consuming the unstamped state (1) clobbers the captured back-jump id with 
`""` → `no loop-back state URI configured for LoopStart ''`, and (2) hands 
`run_update` a State with no `table` payload → `KeyError`. Either way the loop 
breaks. The reviewer reasoned this for a stateful JVM operator; it is 
language-independent (a Python UDF hits the identical path — no JVM built-in 
currently overrides `produceStateOnFinish`, so the Python UDF is also the 
practical repro).
   
   **Fix (consumer-side).** A real loop state is always stamped — the matching 
LoopStart stamps its own id on every iteration's output state — so the LoopEnd 
runtime now keys on the stamp:
   
   | Frame at LoopEnd (counter 0) | before | after |
   |---|---|---|
   | stamped (`loop_start_id` set) | consume + capture id | unchanged |
   | unstamped (`""`) | consume → id clobber / `KeyError` | forward downstream 
unchanged, skip the operator (default pass-through semantics), captured id 
untouched |
   
   Also adds the comment the review asked for at the two Scala boundary-state 
emit sites (`StartChannelHandler` / `EndChannelHandler`), documenting that 
their "no loop" envelope defaults are deliberate and how the LoopEnd runtime 
treats them.
   
   ### Any related issues, documentation, discussions?
   
   Follow-up to #6661 (review discussion r3648708075). Related engine context: 
#6660.
   
   ### How was this PR tested?
   
   - **Unit** (`test_main_loop.py`): new test pins that an unstamped counter-0 
frame at a LoopEnd is forwarded with its envelope unchanged, the operator is 
not invoked, and the captured back-jump id is not clobbered (red before the 
fix); the existing stamped-consume test is unchanged and still passing (95/95 
across the loop suites locally).
   - **E2E** (`LoopIntegrationSpec`, CI-only): new case runs `TextInput → 
LoopStart → stateful Python UDF → LoopEnd` where the UDF emits boundary state 
via `produce_state_on_finish` — it crashes without this fix and completes 
exactly 3 iterations with it.
   - `scalafmtCheckAll` + `scalafixAll --check` + full test-compile + ruff 
format/check pass locally (Java 17).
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (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