Repository navigation
HIVE-29625: Disambiguate ColStatistics.countDistinct "unknown" from "verified zero" - #6505
Open
konstantinb wants to merge 14 commits into
Open
konstantinb wants to merge 14 commits into
konstantinb wants to merge 14 commits into
Conversation
konstantinb
force-pushed
the
HIVE-29625
branch
from
October 6, 2026 19:53
49b60d5 to
68a9747
Compare
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 |
konstantinb
force-pushed
the
HIVE-29625
branch
from
October 7, 2026 14:59
13d9437 to
f11d711
Compare
konstantinb
force-pushed
the
HIVE-29625
branch
from
October 7, 2026 22:25
f11d711 to
12a3f8c
Compare
konstantinb
force-pushed
the
HIVE-29625
branch
from
October 8, 2026 05:34
12a3f8c to
b988af0
Compare
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Reviewer summary: the production change is +199/-100 across 8 files; the other 13 files
(+1,838/-216) are tests. Only two existing
.q.outgoldens change, both because binary columns,whose metastore stats carry no NDV, now report an unknown NDV (
-1) instead of an implicit0that consumers took as a real value. Suggested reading order:
StatsUtils, thenStatisticsand
PessimisticStatCombiner, thenStatsRulesProcFactory.What changes were proposed in this pull request?
HIVE-29625: Disambiguate
ColStatistics.countDistinct"unknown" from "verified zero".Establishes
-1as the "unknown" NDV sentinel forColStatistics.countDistinct, parallel tothe historical convention for
numNulls/numTrues/numFalses, which HIVE-29438 reinforced.Producers emit
-1when the NDV is genuinely unavailable:ColStatisticsconversion returns-1whennumDVsis unset (previously thethrift default
0);explicitly report
-1(tsltz likewise fornumNulls).Consumers of
getCountDistint()distinguish< 0(unknown),== 0(verified zero) and> 0(verified positive). Unknown triggers the same fallback the overloaded0used totrigger; verified zero flows through the math naturally. Two consumer rules are worth calling
out:
getDenominator) compute on the known values whenonly some inputs are unknown — the known NDVs bound the join-key domain from below, so this
reproduces the previous behavior (an unknown-as-
0lost the max for two relations and wasthe 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.
getDenominatorForUnmatchedRows) propagate unknown: an unknown mayresolve 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.addToColumnStatsplus all four fieldmerges in
PessimisticStatCombiner) andStatsUtils.scaleDownNDV(three call sites; alsounifies the row-count-ratio boundary at
< 1.0and keeps NDVs above 2^53 exact by skippingthe double round-trip when the row count does not shrink).
This also fixes HIVE-29556 (
extractNDVGroupingColumnsadjusting an unknown NDV of0to1).Why are the changes needed?
ColStatistics.countDistinct == 0was 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
castexpression with a known NDV could only be estimated as if no keystatistics 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
0distinct values.How was this patch tested?
(
TestStatsUtils,TestStatsRulesProcFactory,TestPessimisticStatCombiner, plus the newoptimizer 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).
.qregression tests:join_ndv_unknown_mixed.q(binary key vscastkey — thejoin 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).
.q.outexpected-result files: only two shift, both traced to the binary NDVproducer change — the
probeDecodeDetailsline invector_binary_join_groupby.qand thebinary GROUP BY estimates in
parquet_types_non_dictionary_encoding_vectorization.q. Allaffected test classes pass.
TestStatsRulesProcFactorynow uses JUnit 5 so it can host the parameterized cases. As part ofthat, its 16 existing min/max comparison tests were folded into four parameterized groups, with
every case's operator, literal and expected row count preserved.