gortiz opened a new issue, #19781:
URL: https://github.com/apache/pinot/issues/19781
## Summary
Column `minValue` and `maxValue` in segment metadata include the placeholder
that Pinot stores for null rows. With
null handling, this makes the values useless as bounds for a column that has
nulls. Today each optimization that
reads min/max adds its own guard for nulls, and most guards simply disable
the optimization.
This issue proposes to fix the cause. We add three optional, sparse keys per
column to `metadata.properties`:
`numNulls`, `minNonNullValue` and `maxNonNullValue`. The existing keys keep
their meaning. Segments created before the
change report "unknown", and the optimizations keep their current
conservative behavior for those segments. A
server-level flag lets operators backfill old segments on reload.
We want feedback on the format, the compatibility rules and the PR plan
before we start.
## Background
The forward index has no null representation. When a value is null,
ingestion writes the `defaultNullValue` of the
column into the forward index and marks the doc in the null value vector.
The stats collectors receive the
placeholder value, so it goes into the dictionary, `minValue`, `maxValue`
and the cardinality.
For numeric dimensions the default is the minimum of the type
(`Integer.MIN_VALUE` for INT). One null row is enough
to set `minValue` to that value. For metrics the default is `0`, which can
also become the min. A custom
`defaultNullValue` does not solve the problem: any value can be an extreme
in some segment.
The existing keys must keep this meaning, for three reasons:
- When null handling is disabled in a query, a null row *is* the default
value. `MIN(col)` from metadata or from the
dictionary must return it.
- The bit-sliced range index uses the stored min as an offset. Its on-disk
layout depends on it.
- The dictionary contains the default value anyway.
## Work that this change would have helped
| PR / issue | Effect of the polluted min/max |
|---|---|
| #19725, fixed by #19724 |
`MinMaxValueBasedSelectionOrderByCombineOperator` returned wrong rows (and an
NPE) with null handling. The fix makes every segment with nulls unskippable
when nulls sort first. |
| #18692 (fix for #18685) | `SelectionQuerySegmentPruner` now prunes
filtered `ORDER BY col LIMIT n` queries. It is disabled when null handling is
active on the order-by or filter column, because nulls pollute min/max. |
| #19120 (open) | The streaming selection order-by combine prunes by min/max
only when `isNonNull` flags the leading column. A segment with any null, or
built before #19072, is always activated. |
| #19072 | Added the `isNonNull` column flag. It covers only the zero-null
case. |
| #14342 | The non-scan `MIN`/`MAX` path of `AggregationPlanNode` is
disabled when null handling is enabled and the segment has nulls. |
With non-null bounds, #19724, #18692 and #19120 can prune segments that have
nulls, instead of always processing
them.
## Proposal
### New keys
All keys are optional and live under `column.<name>.` in
`metadata.properties`:
| Key | When the writer writes it |
|---|---|
| `numNulls` | The column has one or more nulls, and the values below are
representable. |
| `minNonNullValue` | Only when the min of the non-null values is different
from `minValue`. |
| `maxNonNullValue` | Only when the max of the non-null values is different
from `maxValue`. |
The keys are sparse because tables can have hundreds of thousands of
segments. A column without nulls gets no new
key. A column with nulls gets only `numNulls` when the default value is not
an extreme, for example when
`defaultNullValue` is between the real values.
The writer writes all the keys for a column, or none of them. It writes none
when a value is not representable (for
example, a STRING longer than the 512-character metadata limit), or when the
stored min/max is absent. "None" means
"unknown", which is always safe.
### How a reader resolves the values
`NullValueVectorCreator` writes no bitmap file when a column has zero nulls.
As a result, the presence of the bitmap
already means "one or more nulls", in old and new segments. No new marker is
necessary:
| Null bitmap | `numNulls` key | Result |
|---|---|---|
| absent | - | Known. Zero nulls, so the non-null min/max are
`minValue`/`maxValue`. |
| present | present | Known. An absent `minNonNullValue`/`maxNonNullValue`
means "equal to `minValue`/`maxValue`". |
| present | absent | Unknown. The segment is from an older version and has
not been backfilled. |
### Java API
```java
/// Statistics computed over the non-null values of a column in one segment.
/// `minValue` and `maxValue` are null only when every value is null.
public record NonNullStats(int numNulls, int numDocs,
@Nullable Comparable<?> minValue, @Nullable Comparable<?> maxValue) {
public boolean hasNulls() { return numNulls > 0; }
public boolean isAllNull() { return numNulls == numDocs; }
}
// DataSourceMetadata
/// Returns null when the segment cannot supply the values.
@Nullable
default NonNullStats getNonNullStats() { return null; }
```
The immutable data source applies the resolution rules. The mutable data
source tracks the values during
consumption, so it always returns them. Query code selects the variant from
the query: with null handling it uses
`getNonNullStats()`, and without null handling it uses
`getMinValue()`/`getMaxValue()` as today.
### Old segments
A reload does not change old segments by default. `SegmentPreProcessor`
checks every segment when a server loads it,
so an always-on backfill would rewrite every old segment with nulls at
server startup. A server-level
flag, disabled by default, enables the backfill.
The backfill is cheap in most cases. The forward index value at the first
null doc is the default that was used when
the segment was created. If that value is not equal to `minValue` or
`maxValue`, the stored values are already the
non-null bounds, and only `numNulls` (the bitmap cardinality) is written. A
scan is necessary only for the extreme that
the default occupies. For a dictionary column with an inverted index, a
bitmap operation replaces the scan.
The feature is only an optimization. Segments that stay "unknown" keep the
current behavior, and retention removes
them over time.
## Compatibility
- There is no segment format version change. `ColumnMetadataImpl` ignores
unknown keys, so an older server can load
a new segment, and a rollback is safe.
- An older server that adds a null bitmap on reload (#19072) does not write
`numNulls`. The column then resolves to
"unknown".
- The new interface methods are `default` methods and return "unknown".
External implementations of
`ColumnMetadata`, `ColumnStatistics` and `DataSourceMetadata` still
compile.
- The new getters appear in the JSON of the server segment metadata REST
API. This change only adds fields.
## Planned PRs
Each PR keeps "unknown" as the fallback. Thus the PRs can merge in this
order without a risk of wrong results.
1. SPI, reader, writer and offline creation. Add `NonNullStats` and the
keys. Pass the null state of each row to the
stats collectors (row and columnar paths), which keep a running non-null
min/max. Write the keys in
`BaseSegmentCreator`, and apply the resolution rules in
`ImmutableDataSource`.
2. Realtime and default columns. Track non-null min/max in
`MutableSegmentImpl`, and pass them through the
realtime-to-offline converter statistics. Compacted segments compute them
from valid docs only. Handle new default
columns and derived columns in `BaseDefaultColumnHandler`. Make sure that
every path that rewrites column metadata
also rewrites or removes the new keys.
3. Backfill on reload, behind a server-level flag that is disabled by
default.
4. `MinMaxValueBasedSelectionOrderByCombineOperator`. With null handling,
use the non-null bounds, and treat a segment
as unskippable for nulls-first only when it has nulls. Keep the #19724
behavior for "unknown" segments.
5. `SelectionQuerySegmentPruner`. Enable it with null handling when every
segment involved supplies non-null stats.
6. Optional: `ColumnValueSegmentPruner`. With null handling, EQ, IN and
range predicates do not match null docs, so the
pruner can use the non-null bounds.
## Out of scope
- `startTime` and `endTime` in ZooKeeper. A null time value also changes
them, but broker pruning on non-null bounds
would break `WHERE ts IS NULL`.
- Non-scan `MIN`/`MAX` with null handling. A future aggregation function for
non-null min/max can use the new API.
- Storing `defaultNullValue` in the metadata. The forward index value at the
first null doc gives it.
## Questions for the community
1. Are the sparse keys and the "bitmap present but no `numNulls` means
unknown" rule acceptable, or do you prefer
explicit keys on every column?
2. Must the backfill flag be at server level or at table level?
3. Do you know other consumers of min/max that need a null guard today?
--
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]
