github-actions[bot] commented on code in PR #65483:
URL: https://github.com/apache/doris/pull/65483#discussion_r3627560787
##########
be/src/core/value/bitmap_value.h:
##########
@@ -2281,6 +2281,67 @@ class BitmapValue {
return 0;
}
+ uint64_t xor_cardinality(const BitmapValue& rhs) const {
+ switch (rhs._type) {
+ case EMPTY:
+ return cardinality();
+ case SINGLE:
+ switch (_type) {
+ case EMPTY:
+ return 1;
+ case SINGLE:
+ return 1 + (_sv != rhs._sv);
Review Comment:
[P1] Return zero for equal singleton XORs
When both operands use the `SINGLE` representation and contain the same
value, their symmetric difference is empty, but this expression returns `1 +
false == 1`. The new `bitmap_xor_count` shortcut calls `xor_cardinality()`
directly, so with `enable_set_in_bitmap_value=false`,
`bitmap_xor_count(to_bitmap(k), to_bitmap(k))` (and the rewritten
count-over-XOR form) reaches equal `SINGLE` operands and reports 1 instead of
0. The current representation-matrix test only compares singleton 1 with
singleton 2, so please add the equal case as well.
```suggestion
return 2 * (_sv != rhs._sv);
```
##########
fe/fe-core/src/main/java/org/apache/doris/qe/CoordinatorContext.java:
##########
@@ -310,6 +310,7 @@ public static CoordinatorContext buildForLoad(
queryOptions.setBeExecVersion(Config.be_exec_version);
queryOptions.setNewVersionUnixTimestamp(true);
queryOptions.setNewVersionPercentile(true);
+ queryOptions.setNewVersionBitmapOpCount(true);
Review Comment:
[P1] Propagate the flag through the legacy broker-load coordinator
This updates the Nereids load-options builder, but the cloud broker-load
fallback still reaches `CloudCoordinator(Long, ...)` / `Coordinator(Long, ...)`
when the global `enable_nereids_distribute_planner` option is disabled. That
parallel constructor creates a bare `TQueryOptions` without this flag. For a
mapping such as `dst = bitmap_and_count(bitmap_from_string(src1),
bitmap_from_string(src2))`, the nullable inputs make legacy BE derive
`Nullable(BIGINT)` while the new FE serialized non-null `BIGINT`, so function
preparation rejects the load. This is separate from the earlier short-circuit
thread, whose planner options already contain the flag. Please set or
centralize the version option in the legacy load coordinator too and cover this
cloud fallback.
--
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]