Skip to content

HIVE-29625: Disambiguate ColStatistics.countDistinct "unknown" from "verified zero" - #6505

Open
konstantinb wants to merge 14 commits into
apache:masterfrom
konstantinb:HIVE-29625
Open

konstantinb wants to merge 14 commits into
apache:masterfrom
konstantinb:HIVE-29625

Conversation

@konstantinb

@konstantinb konstantinb commented May 21, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer summary: the production change is +199/-100 across 8 files; the other 13 files
(+1,838/-216) are tests. Only two existing .q.out goldens change, both because binary columns,
whose metastore stats carry no NDV, now report an unknown NDV (-1) instead of an implicit 0
that consumers took as a real value. Suggested reading order: StatsUtils, then Statistics
and PessimisticStatCombiner, then StatsRulesProcFactory.

What changes were proposed in this pull request?

HIVE-29625: Disambiguate ColStatistics.countDistinct "unknown" from "verified zero".

Establishes -1 as the "unknown" NDV sentinel for ColStatistics.countDistinct, parallel to
the historical convention for numNulls/numTrues/numFalses, which HIVE-29438 reinforced.

Producers emit -1 when the NDV is genuinely unavailable:

  • proto-to-ColStatistics conversion returns -1 when numDVs is unset (previously the
    thrift default 0);
  • binary and timestamp-with-local-time-zone columns, whose stats data carries no NDV, now
    explicitly report -1 (tsltz likewise for numNulls).

Consumers of getCountDistint() distinguish < 0 (unknown), == 0 (verified zero) and
> 0 (verified positive). Unknown triggers the same fallback the overloaded 0 used to
trigger; verified zero flows through the math naturally. Two consumer rules are worth calling
out:

  • Max/product-style join denominators (getDenominator) compute on the known values when
    only some inputs are unknown — the known NDVs bound the join-key domain from below, so this
    reproduces the previous behavior (an unknown-as-0 lost the max for two relations and was
    the excluded least value for three or more) instead of collapsing to a cross-product
    estimate. Only an all-unknown input makes the denominator itself unknown.
  • Min-style combines (getDenominatorForUnmatchedRows) propagate unknown: an unknown may
    resolve below every known value, so no known value is a safe bound.

Two small shared helpers consolidate previously duplicated logic:
StatsUtils.maxOrUnknown (five call sites: Statistics.addToColumnStats plus all four field
merges in PessimisticStatCombiner) and StatsUtils.scaleDownNDV (three call sites; also
unifies the row-count-ratio boundary at < 1.0 and keeps NDVs above 2^53 exact by skipping
the double round-trip when the row count does not shrink).

This also fixes HIVE-29556 (extractNDVGroupingColumns adjusting an unknown NDV of 0 to 1).

Why are the changes needed?

ColStatistics.countDistinct == 0 was overloaded to mean both "no distinct values" and
"unknown NDV". The two states have opposite implications for cost-based planning — a real zero
supports tight estimates, while a genuinely-unknown NDV needs a conservative fallback. Treating
them identically led to inconsistent and sometimes severe cardinality estimates whenever a
column's NDV was unavailable: for example, a join between a table with a binary key (NDV always
unavailable) and a cast expression with a known NDV could only be estimated as if no key
statistics existed at all. Disambiguating the sentinel lets each consumer apply the correct
logic.

@zabetak noted during review of #6359: "In fact, everything would be simpler if we could use -1
for NDV to declare unknown as it happens for the other stats. This is probably a bigger change to
digest so let's not go into this direction for now."
(#6359 (comment)) This PR takes up that bigger
change.

Does this PR introduce any user-facing change?

No. Query results, SQL syntax, and configuration are unchanged. Plan estimates in EXPLAIN
output may differ for queries reading columns whose NDV is unavailable, since the planner now
distinguishes those from columns with 0 distinct values.

How was this patch tested?

  • Parameterized unit-test matrices for every producer branch and consumer rule
    (TestStatsUtils, TestStatsRulesProcFactory, TestPessimisticStatCombiner, plus the new
    optimizer tests under ql/src/test/.../optimizer/), covering unknown / verified-zero /
    verified-positive for each site and the row-count-ratio boundary (including NDVs above 2^53).
  • Two new .q regression tests: join_ndv_unknown_mixed.q (binary key vs cast key — the
    join estimate stays at parity with the previous behavior instead of degrading to a
    cross product) and join_ndv_unknown_mixed_multikey.q (the correlated multi-key branch,
    inner and left-outer join).
  • Existing .q.out expected-result files: only two shift, both traced to the binary NDV
    producer change — the probeDecodeDetails line in vector_binary_join_groupby.q and the
    binary GROUP BY estimates in parquet_types_non_dictionary_encoding_vectorization.q. All
    affected test classes pass.
  • TestStatsRulesProcFactory now uses JUnit 5 so it can host the parameterized cases. As part of
    that, its 16 existing min/max comparison tests were folded into four parameterized groups, with
    every case's operator, literal and expected row count preserved.

@konstantinb

Copy link
Copy Markdown
Contributor Author

@kasakrisz would you have time to review this one? You reviewed and merged #6423, and last month you merged HIVE-29365 in the stats-estimation area. The production change is +199/-100 across 8 files; the other 13 files are tests, and only two existing goldens change.

@thomasrebele a first-pass review from you would also be very welcome, since this touches FilterStatsRule, which your HIVE-29442 and HIVE-29300 reworked.

@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2026

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants