HappenLee opened a new pull request, #68219:
URL: https://github.com/apache/doris/pull/68219
### What problem does this PR solve?
Problem Summary:
`bitmap_agg` and `bitmap_union_int` currently pass signed negative integers
into `BitmapValue`, which converts them to unsigned bitmap members. For
example, `bitmap_agg(-1)` can contain `18446744073709551615` and
`bitmap_union_int(-1)` returns 1, while `to_bitmap(-1)` produces an empty
bitmap.
Filter negative integers before insertion in both aggregates. Cover scalar
insertion, nullable and non-nullable `bitmap_agg` batches, and the integer-only
branch of the shared bitmap-count aggregate. Preserve the allocation-free batch
path for all-nonnegative input and use a tracked temporary buffer for filtered
batches. Generic bitmap operations and the full uint64 domain of
`bitmap_union_count` are unchanged.
With this change:
- `bitmap_count(bitmap_agg(-1))` and `bitmap_union_int(-1)` return 0.
- Input `[-1, 0, 1, 1, NULL]` contributes exactly the members `{0, 1}`.
- All-negative/all-NULL groups produce an empty bitmap or a count of 0.
### Release note
`bitmap_agg` and `bitmap_union_int` now ignore negative integers,
consistently with `to_bitmap`. In particular, `bitmap_union_int` counts
distinct nonnegative integers rather than all distinct integers. Queries
containing negative inputs can return smaller counts after upgrading.
### Check List (For Author)
- Test: add BE unit tests covering TINYINT/SMALLINT/INT/BIGINT,
nullable/non-nullable input, negative minima, positive maxima, zero,
duplicates, all-negative/all-NULL groups, dense batches, reset, streaming
serialization and column merge paths. Also protect UINT64_MAX in
`bitmap_union_count`.
- Passed: clang-format 16 check, BE build-hygiene checks and `git diff
--check`.
- Passed: compiler syntax checks of both aggregate
registration/instantiation translation units and the modified test translation
unit, using the generated ASAN-UT compilation settings.
- Attempted: `run-be-ut.sh -j48 --run
--filter='BitmapIntegerAggregateTest.*:AggBitmapTest.*'`. The build stops in
unchanged `common/cpp/aws_common.cpp` because the local thirdparty installation
lacks `aws/core/auth/GeneralHTTPCredentialsProvider.h`; tests have not executed
locally.
- Attempted: repository clang-tidy script. Analysis is blocked by an
existing unmatched NOLINTEND in `core/types.h` and existing warnings in the
touched files outside the new logic. No clean clang-tidy run is claimed.
- Behavior changed: Yes, negative integers no longer contribute bitmap
members or counts. NULL handling, result types, function signatures and
intermediate/storage formats remain unchanged.
- Does this need documentation: Yes, companion English/Chinese
development-version documentation updates clarify the changed
`bitmap_union_int` semantics.
### Review notes
The existing aggregate test helper exercises the batch, streaming and merge
paths without adding a new framework. Grouped additions use the same scalar
`add` entry point. No new locks, configuration, persistence changes or FE/BE
fields are introduced. Filtering is applied only to integer inputs;
already-materialized bitmaps are preserved. During a rolling upgrade, old BEs
can still contribute old partial states, so uniform negative-input semantics
require all executing BEs to contain this fix.
--
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]
