You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "minni31 (via GitHub)" <gi...@apache.org> on 2026/07/27 08:23:23 UTC

[I] [SQL] Clarify bitmap_or_agg / bitmap_and_agg input contract: fixed 4096-byte vs. variable-length bitmaps [spark]

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: reviews-unsubscribe@spark.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [I] [SQL] Clarify bitmap_or_agg / bitmap_and_agg input contract: fixed 4096-byte vs. variable-length bitmaps [spark]

Posted by "ybapat (via GitHub)" <gi...@apache.org>.
ybapat commented on issue #57561:
URL: https://github.com/apache/spark/issues/57561#issuecomment-5093090263

   I've opened a PR that adds runtime size validation to both aggregates: #57567
   
   The fix throws a `SparkRuntimeException` (`BITMAP_INPUT_TOO_LARGE`, SQLSTATE 22001) in `BitmapOrAgg.update()` and `BitmapAndAgg.update()` when the input exceeds 4096 bytes, rather than silently truncating.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [I] [SQL] Clarify bitmap_or_agg / bitmap_and_agg input contract: fixed 4096-byte vs. variable-length bitmaps [spark]

Posted by "minni31 (via GitHub)" <gi...@apache.org>.
minni31 commented on issue #57561:
URL: https://github.com/apache/spark/issues/57561#issuecomment-5129350771

   Perfect, It seems to be handling same issue. Thanks for the patch.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org