yinli-systems commented on PR #11062:
URL: https://github.com/apache/arrow-rs/pull/11062#issuecomment-5800037346

   I traced the Dictionary path end to end rather than assuming the copied 
planner branch meant it worked. It currently does not: Dictionary finite-offset 
RANGE is a separate silent-correctness bug in DataFusion.
   
   On DataFusion b300cea226 (with Arrow 59.3), I ran the same four-row query 
twice:
   
   - dense ORDER BY Int64: 20, 45, 69, 47
   - ORDER BY arrow_cast(temperature, 'Dictionary(Int32, Int64)'): 89, 89, 89, 
89
   
   The probe passes when those exact outputs are asserted. It does not fall 
back or report an error.
   
   The call path explains the result:
   
   1. extract_window_frame_target_type unwraps Dictionary only to coerce the 
frame offset to Int64. This is the part #24565 copied for REE.
   2. At execution, get_row_at_idx / ScalarValue::try_from_array retains the 
wrapper as ScalarValue::Dictionary(...), just as it retains 
ScalarValue::RunEndEncoded(...).
   3. ScalarValue::add_checked / sub_checked delegates the wrapped one-row 
array to arrow_arith::numeric.
   4. numeric arithmetic has neither Dictionary nor (without this PR) REE 
dispatch.
   5. WindowFrameStateRange currently treats every arithmetic error as overflow 
and returns an unbounded partition edge. That turns the unsupported Dictionary 
operation into a full-partition frame, hence 89 for every row.
   
   So Dictionary is not an existing working precedent that DataFusion uses 
differently. The planner accepts it, but the runtime result is wrong. REE has 
the same wrapper-preserving path; #11062 makes that path produce the correct 
20, 45, 69, 47 result while retaining the run representation. Dictionary 
arithmetic remains a distinct missing-kernel/DataFusion-error-classification 
issue and should not be claimed as solved here.
   
   This corrects the ambiguity in my earlier DataFusion comparison. I can file 
the Dictionary behavior separately, but I would keep this PR scoped to the 
accepted #10891 REE kernel work unless you prefer the two encodings handled 
together.


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