Doris-Breakwater commented on issue #66033:
URL: https://github.com/apache/doris/issues/66033#issuecomment-5076678709
Breakwater-GitHub-Analysis-Slot: slot_b8443b755acb
## Initial triage
**Verdict: confirmed BE correctness bug; the root cause is sufficiently
established from the code path.** No production profile is needed for this
issue.
The vulnerable expression is present in current upstream `master` and
`branch-4.0`:
```cpp
max_step = std::min(max_step, (int)(_cur_size - _cur_offset));
```
`BitmapValue::cardinality()` returns `uint64_t`, while `TableFunction` keeps
`_cur_size` and `_cur_offset` as `int64_t`. `TableFunctionOperator` passes a
positive `max_step` bounded by the remaining output-block capacity, but
`VExplodeBitmapTableFunction::get_value()` narrows the potentially much larger
remaining cardinality *before* applying that bound.
For a remaining cardinality of `INT_MAX + 1` on the current 64-bit ABI, the
narrowing produces `INT_MIN`. In the non-nullable path this is converted to a
huge `size_t` by `target->resize(origin_size + max_step)`; in the nullable path
the null-map `insert_many_defaults(max_step)` can fail first for the same
reason. The exact allocator/check that terminates the process can vary, but the
invalid row count and resulting BE failure are deterministic consequences of
this code. For some other cardinalities the narrowing can become zero (for
example, `2^32` on the current ABI), causing the operator loop to make no
progress instead of immediately failing.
The linked PR, #66034, compares in `int64_t` space and only then casts the
result:
```cpp
max_step =
static_cast<int>(std::min(static_cast<int64_t>(max_step), _cur_size
- _cur_offset));
```
That fix is logically correct for this path: the result is non-negative by
the table-function cursor invariant and cannot exceed the caller's `int
max_step`, so the final cast is representable. It fixes both the negative-size
crash and the zero-progress case.
## Missing information
Nothing is blocking root-cause confirmation or review of the fix. For
release/backport tracking, it would still be useful for the reporter to provide:
- the exact Doris build/commit within 4.0 rather than only `master / 4.0`;
- the top of the BE core backtrace and whether the output column was
nullable, to record which allocation path failed first.
These should not block the fix because both current upstream branches
contain the vulnerable expression.
## Recommended next steps
1. Continue review of #66034, but request a regression unit test before
merge. The PR's claim that the boundary necessarily needs roughly 16 GB is not
generally true for Roaring bitmaps. A run-optimized contiguous range above
`INT_MAX` is compact: a local Roaring boundary probe with cardinality
`2,147,483,649` serialized to about 463 KB and used about 57 MB RSS. A BE test
can construct/deserialize a run-optimized `BITMAP32` covering `[0, INT_MAX +
1)`, call `process_row()`, request only a small batch from `get_value()`, and
assert a positive returned step, the first emitted values, and `!eos()`. It
does not need to enumerate all bitmap elements.
2. After the master fix is merged, backport it to `branch-4.0`; the same
line is confirmed there in the older
`be/src/vec/exprs/table_function/vexplode_bitmap.cpp` path.
3. Audit the identical narrow-before-`std::min` pattern in sibling table
functions (`vexplode`, `vexplode_v2`, `vexplode_map`, Java/Python UDTFs, and
JSON table functions). Reachability differs by input type, but array/map
lengths also use 64-bit offsets. A shared checked/clamped helper would reduce
the chance of fixing only the bitmap instance.
4. Add the project's bug and BE/query-table-function labels, plus an
affected-4.0 label if that is part of the repository's labeling convention. The
issue currently has no labels.
Suggested triage priority: **medium-high**. Triggering the boundary is
uncommon, but the impact is a deterministic BE process failure (or
non-progress) from valid bitmap input, and a narrow, low-risk fix is already
available.
--
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]
