minni31 opened a new issue, #57561: URL: https://github.com/apache/spark/issues/57561
### What The documented contract for `bitmap_or_agg` (and `bitmap_and_agg`) is internally contradictory about what inputs are valid, and Spark's implementation silently accepts inputs the docs imply are invalid. We'd like the intended contract clarified so downstream engines can match it faithfully. ### The contradiction `BitmapOrAgg`'s `@ExpressionDescription` (in `sql/catalyst/.../expressions/bitmapExpressions.scala`) states: > "The input should be bitmaps created from `bitmap_construct_agg()`." `bitmap_construct_agg` always produces a fixed **4096-byte** bitmap (`BitmapExpressionUtils.NUM_BYTES`). Yet the **documented examples for the same function** feed 1-byte literals: ```sql SELECT substring(hex(bitmap_or_agg(col)), 0, 6) FROM VALUES (X '10'), (X '20'), (X '40') AS tab(col); -- 700000 ``` These 1-byte values are not produced by `bitmap_construct_agg`. Implementation-wise, `update()`/`merge()` call `BitmapExpressionUtils.bitmapMerge`, which ORs only `min(buffer.length, input.length)` bytes into a fixed 4096-byte buffer. So: - inputs **shorter** than 4096 bytes merge only their bytes (the rest stay zero), and - inputs **longer** than 4096 bytes are **silently truncated** to the first 4096. ### Questions 1. Is raw user-supplied `VARBINARY` (i.e. not the output of `bitmap_construct_agg`) a **supported** input to `bitmap_or_agg` / `bitmap_and_agg`, or is the `bitmap_construct_agg`-only sentence the real contract? 2. For inputs **shorter** than 4096 bytes — is the current "merge only the provided bytes" behavior intended (as the examples suggest), or should it be an error? 3. For inputs **longer** than 4096 bytes — is the silent `min()` truncation intended, or should it error/validate? 4. If variable-length input is intended, can the `@ExpressionDescription` be updated so the "should be bitmaps created from `bitmap_construct_agg()`" line no longer contradicts the examples and the `min()` merge? ### Why we're asking Apache Gluten + [Velox](https://github.com/facebookincubator/velox) are implementing these Spark aggregates natively and need to match Spark's exact semantics. A Velox reviewer flagged that matching the current `min()` behavior risks baking in possibly-accidental behavior ([facebookincubator/velox#18045](https://github.com/facebookincubator/velox/pull/18045)). Clarifying the intended contract here lets us implement it correctly rather than guess. Affected: `bitmap_or_agg`, `bitmap_and_agg` (since 3.5.0). -- 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]
