Jackie-Jiang opened a new pull request, #19390:
URL: https://github.com/apache/pinot/pull/19390
## Summary
With null handling enabled, a group-by query was routed to the no-dictionary
group key generators regardless of how its columns are encoded, and the
multi-value key path ignored nulls entirely. This PR makes every group key
generator null-aware, so the routing can go back to being decided by column
encoding alone.
### Dictionary-based group key generation
With null handling enabled, a column whose segment tracks nulls reserves one
further dictionary id, one past its last real id, to stand for a null value;
its cardinality counts one more than the dictionary holds. A block's null rows
are moved onto the reserved id before key composition (copying the block's id
array, which is shared with other readers of the column), so the per-row
kernels and the raw key arithmetic are untouched. The reserved id composes into
the raw key like any other value and is read back out as SQL `NULL`.
A column that tracks no nulls reserves nothing, and neither does any column
when null handling is disabled, which leaves the cardinalities, the holder
selection and the block reads exactly as they are today. Dictionary-encoded
group-by columns therefore keep the dictionary-based generator in both modes,
instead of paying for per-row hash lookups on the no-dictionary path.
### Multi-value key path of the no-dictionary generators
The `int[][]` overloads of both no-dictionary generators read every column
without consulting its null bitmap, so a null row was grouped under the
column's default null value instead of under a group of its own — for every
column on that path, single-value columns included. Both overloads now
recognize nulls: a null row contributes one `NULL` group, mirroring the
single-value behavior and the row's physical storage as a one-element default
value.
With that closed, the null-enabled and null-disabled twin loops in both
generators are merged: every iteration on these paths pays for a hash-map
operation, which dwarfs the predicted per-row null check the merge adds, and
both files end up smaller than before while doing more.
### Bugfix: rows misattributed once the group limit is reached
The null-enabled at-limit path of `NoDictionaryMultiColumnGroupKeyGenerator`
resolved each column's key value but never stored it, composing whatever the
key buffer held from the previous row — which, after a new group's buffer swap,
is `{0, 0, ...}`: exactly the first group's key. With null handling enabled and
`numGroupsLimit` reached, rows belonging to existing groups could be counted
into the first group or dropped. The resolved value is now stored, same as the
null-disabled twin always did.
### Behavior changes
All confined to queries that enable null handling:
- Grouping by a multi-value column with null rows now returns a `NULL`
group; previously those rows were grouped under the column's default null value.
- Rows past the group limit are attributed to their correct existing groups
(the bugfix above).
- The group id upper bound for dictionary-encoded columns is now derived
from cardinalities rather than defaulting to `numGroupsLimit`, matching the
null-handling-disabled behavior.
With null handling disabled nothing changes: no id is reserved, the routing
condition is the encoding check it used to be, and the per-row loops of the
dictionary-based generator are byte-identical.
### Tests
`NullHandlingEnabledQueriesTest` gains nine query-level tests covering every
generator and key path with nulls: dictionary-encoded SV (including a row
holding the column's default null value, which must not join the `NULL` group),
two dictionary columns, dictionary MV, raw SV (INT and STRING, which exercise
the primitive-map and object-map null keys respectively), raw MV, mixed
raw/dictionary SV and MV, and a regression test for the group-limit
misattribution that fails on the previous code by construction.
`DictionaryBasedGroupKeyGeneratorTest` additionally covers
`IntGroupIdMap.clearAndTrim` past the caching threshold.
--
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]
