apache / apache/spark

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

Open
#57,561 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Scala
Stars
44k
Forks
29.4k
PR merge metrics
No merged PRs in 30d

Description

### 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).

Contributor guide

Open the contributing guide

Research direction

Read sql/catalyst/.../expressions/bitmapExpressions.scala and BitmapExpressionUtils.bitmapMerge first; compare the @ExpressionDescription text with the 1-byte examples and the fixed-buffer behavior described here. Done means the supported input contract for both aggregates is decided and the expression documentation and any required validation behavior are made consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala, sql
Domain
databases, documentation
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.