[SQL] Never place two retain_value operators on the same input operator -- insert a noop on one side instead#6355
Conversation
…or -- insert a noop on one side instead Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
mythical-fred
left a comment
There was a problem hiding this comment.
Solid targeted fix for #6350. The record + noop-when- cleanly prevents two operators on the same upstream op, and the placeholder in the left-side iomap walk (InsertLimiters.java:1290-1296) is the real reason the field-order swap (issue6350b) used to crash: previously skipped unused fields entirely, leaving / shorter than and producing a whose shape did not match the left input type. The two regression tests cover both reproductions and assert exactly 2 operators, which is the right shape.
A few non-blocking questions inline. Approving — please consider them before merge.
|
(Sorry — my APPROVE review body above lost all its code spans due to a quoting bug on my side. Re-posting the intended body here; inline comments have been corrected in place.) Solid targeted fix for #6350. The Approval stands; non-blocking follow-ups are inline. |
ryzhyk
left a comment
There was a problem hiding this comment.
Ideally, we would merge the two bounds, but I think this is sound.
There may be a better way to resolve this, but for now we took a conservative approach and we create something like this:
Describe Manual Test Plan
Ran all Java tests manually.
Checklist