HIVE-29625: Disambiguate ColStatistics.countDistinct "unknown" from "verified zero" by konstantinb · Pull Request #6505 · apache/hive

konstantinb · 2026-05-21T22:54:01Z

Reviewer summary: the production change is +200/-102 across 8 files; the other 17 files
(+2,216/-211) 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).
Four new .q regression tests: join_ndv_unknown_mixed.q (mixed known/unknown join-key
NDVs), join_ndv_unknown_mixed_multikey.q (correlated multi-key inner and left-outer joins),
groupby_ndv_unknown_multikey.q (preserving master's hash-aggregation sizing decision for
unknown NDVs), and filter_ndv_verified_zero_null_safe.q (null-safe equality against NULL
with a verified-zero NDV).
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.

…verified zero"

konstantinb · 2026-10-06T23:51:46Z

@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.

Copilot

🟡 Changes recommended

Unresolved statistics-estimation defects inflate multi-key group counts and mishandle all-NULL filters, producing incorrect cardinality estimates and potentially inappropriate aggregation choices.

3 open issues

What changed in this PR

This PR separates unknown distinct-value counts (-1) from verified zero in Hive’s statistics pipeline, allowing planner rules to apply different estimates for missing and empty domains.

Changes:

Updates statistics producers, merging, and scaling to preserve unknown NDVs.
Adjusts filter, join, grouping, and partition-planning consumers.
Adds parameterized unit tests and join regressions, with two existing binary-column plan outputs updated.

File	Description
`ql/src/test/results/clientpositive/llap/vector_binary_join_groupby.q.out`	Updates binary-key probe ratio.
`ql/src/test/results/clientpositive/llap/parquet_types_non_dictionary_encoding_vectorization.q.out`	Updates binary GROUP BY estimates.
`ql/src/test/results/clientpositive/llap/join_ndv_unknown_mixed.q.out`	Records mixed-NDV join estimates.
`ql/src/test/results/clientpositive/llap/join_ndv_unknown_mixed_multikey.q.out`	Records multi-key inner and outer join estimates.
`ql/src/test/queries/clientpositive/join_ndv_unknown_mixed.q`	Adds a mixed known/unknown join regression.
`ql/src/test/queries/clientpositive/join_ndv_unknown_mixed_multikey.q`	Adds correlated multi-key join regressions.
`ql/src/test/org/apache/hadoop/hive/ql/stats/TestStatsUtils.java`	Tests NDV production, merging, grouping, and scaling.
`ql/src/test/org/apache/hadoop/hive/ql/stats/estimator/TestPessimisticStatCombiner.java`	Tests unknown NDV propagation.
`ql/src/test/org/apache/hadoop/hive/ql/optimizer/TestSortedDynPartitionOptimizer.java`	Tests partition-cardinality handling.
`ql/src/test/org/apache/hadoop/hive/ql/optimizer/TestSetHashGroupByMinReduction.java`	Tests aggregation-reduction decisions.
`ql/src/test/org/apache/hadoop/hive/ql/optimizer/TestReduceSinkMapJoinProc.java`	Tests hash-join key-count estimates.
`ql/src/test/org/apache/hadoop/hive/ql/optimizer/stats/annotation/TestStatsRulesProcFactory.java`	Migrates to JUnit 5 and expands statistics-rule coverage.
`ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/stats/TestHiveRelMdDistinctRowCount.java`	Tests distinct-row metadata boundaries.
`ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java`	Introduces unknown-aware producers and shared helpers.
`ql/src/java/org/apache/hadoop/hive/ql/stats/estimator/PessimisticStatCombiner.java`	Uses unknown-aware maximum merging.
`ql/src/java/org/apache/hadoop/hive/ql/plan/Statistics.java`	Preserves unknown NDVs during merges.
`ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java`	Updates filter, grouping, and join estimates.
`ql/src/java/org/apache/hadoop/hive/ql/optimizer/SortedDynPartitionOptimizer.java`	Treats negative NDVs as unavailable statistics.
`ql/src/java/org/apache/hadoop/hive/ql/optimizer/SetHashGroupByMinReduction.java`	Preserves configured reduction for unknown NDVs.
`ql/src/java/org/apache/hadoop/hive/ql/optimizer/ReduceSinkMapJoinProc.java`	Separates unknown and zero key counts.
`ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/stats/HiveRelMdDistinctRowCount.java`	Prevents negative distinct-row metadata.

_{Review effort: Balanced}

konstantinb · 2026-11-05T01:22:13Z

+          factor = 0;
+          break;


Keeping the clamp is intentional: hive.stats.filter.in.min.ratio is an opt-in floor for IN estimates without a usable value range, and a stored NDV of 0 is the kind of estimate it protects against. 17bccf8 rewrites the comment that overstated this and pins the interaction in TestStatsRulesProcFactory.

konstantinb · 2026-11-05T01:20:31Z

+      if (dvs == 0) {
+        return 0;


Fixed in dcaac90 for the case this PR introduced: a verified-zero NDV means every row is NULL, so col <=> NULL (either operand order) now keeps all rows. Unknown and known NDVs keep master's estimates; using numNulls there too would change master's behavior, so that's left for a follow-up.

konstantinb · 2026-11-05T01:19:14Z

+              }
+              ndv = parentNumRows / 2;
+            }
+            numEstimatedRows = StatsUtils.safeMult(numEstimatedRows, ndv);


Agreed: this changed master's decision, including for a single unknown key on a large input. Fixed in d69875a: an unknown NDV now keeps master's outcome (unknown was stored as 0, so the estimated hash table was empty), and the known-NDV product, which predates this PR, is unchanged.

…inal PR intent

….in.min.ratio

thomasrebele

Thank you for the PR, @konstantinb! Sorry for the late reply, this has been on my TODO list for a long time.

This PR doesn't seem to change FilterStatsRule itself. It's quite related, so I had a first look. I had a look at the non-test files, and the changes that I understood looked reasonable (I need to take another deeper look at StatsRulesProcFactory). I wonder whether this PR covers all parts of the code that need to be changed. I would still need to check that.

thomasrebele · 2026-11-11T22:51:08Z

+        return numRows / 2;
+      }
+      if (dvs == 0) {
+        // no non-null values: only col <=> NULL can match, and then every row does


Could you explain this? What does <=> mean? "not equals"? If that's the case I would prefer using !=.

Could you explain this? What does <=> mean? "not equals"? If that's the case I would prefer using !=.

<=> is Hive's null-safe equality operator (IS NOT DISTINCT FROM), not inequality; I've clarified the comment while retaining the SQL notation. This branch addresses Copilot's earlier finding: with a verified-zero NDV, every row is NULL, so col <=> NULL must keep every row.

sonarqubecloud · 2026-11-13T16:35:39Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

asf-ci-hive added tests pending tests unstable and removed tests pending tests unstable labels May 21, 2026

konstantinb added 3 commits May 25, 2026 08:31

HIVE-29625: Disambiguate ColStatistics.countDistinct "unknown" from "…

bed032d

…verified zero"

HIVE-29625: impacted .out files + reverting an unintended edit

124abe6

HIVE-29625: itest code, small tweaks + better code reuse

f5b19e9

konstantinb force-pushed the HIVE-29625 branch from 43f74c2 to f5b19e9 Compare May 25, 2026 15:48

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending labels May 25, 2026

HIVE-29625: SQ feedback + better test code

349be2f

asf-ci-hive added tests pending tests passed and removed tests passed tests pending labels May 27, 2026

HIVE-29625: more SQ feedback

03960fd

asf-ci-hive added tests pending tests unstable and removed tests passed tests pending labels May 28, 2026

Merge branch 'master' into HIVE-29625

5b2bc13

asf-ci-hive added tests pending tests unstable and removed tests unstable tests pending labels May 28, 2026

asf-ci-hive added the tests passed label Oct 6, 2026

asf-ci-hive added tests pending tests failed and removed tests passed tests pending labels Oct 7, 2026

konstantinb force-pushed the HIVE-29625 branch from 13d9437 to f11d711 Compare October 7, 2026 14:59

asf-ci-hive added tests pending tests failed and removed tests failed tests pending labels Oct 7, 2026

konstantinb force-pushed the HIVE-29625 branch from f11d711 to 12a3f8c Compare October 7, 2026 22:25

asf-ci-hive added tests pending tests failed and removed tests failed tests pending labels Oct 7, 2026

HIVE-29625: self-identified code nits

b988af0

konstantinb force-pushed the HIVE-29625 branch from 12a3f8c to b988af0 Compare October 8, 2026 05:34

asf-ci-hive removed the tests failed label Oct 8, 2026

HIVE-29625: trigger CI re-run

6c5989b

Copilot AI reviewed Nov 3, 2026

View reviewed changes

konstantinb added 5 commits November 4, 2026 17:30

HIVE-29625: fixed checkMapSideAggregation()'s deviation from the orig…

d69875a

…inal PR intent

HIVE-29625: fix "col <=> NULL" estimate when the NDV is a verified zero

dcaac90

HIVE-29625: clarify verified-zero IN estimate under hive.stats.filter…

17bccf8

….in.min.ratio

Merge branch 'master' into HIVE-29625

e2adeaf

HIVE-29625: SQ feedback

8ba92be

thomasrebele reviewed Nov 11, 2026

View reviewed changes

konstantinb added 2 commits November 12, 2026 11:47

HIVE-29625: clarifying null-safe equality operator

eda2c8d

Merge branch 'master' into HIVE-29625

8a42cfb

Navigation Menu

Repository navigation

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

HIVE-29625: Disambiguate ColStatistics.countDistinct "unknown" from "verified zero"#6505
konstantinb wants to merge 22 commits into
apache:masterfrom
konstantinb:HIVE-29625

konstantinb commented May 21, 2026 •

edited

Loading

Uh oh!

konstantinb commented Oct 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

konstantinb Nov 5, 2026

Uh oh!

konstantinb Nov 5, 2026 •

edited

Loading

Uh oh!

konstantinb Nov 5, 2026

Uh oh!

thomasrebele left a comment

Uh oh!

thomasrebele Nov 11, 2026

Uh oh!

konstantinb Nov 12, 2026

Uh oh!

sonarqubecloud Bot commented Nov 13, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

konstantinb commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

konstantinb commented Oct 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

konstantinb Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

konstantinb Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konstantinb Nov 5, 2026

Choose a reason for hiding this comment

Uh oh!

thomasrebele left a comment

Choose a reason for hiding this comment

Uh oh!

thomasrebele Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

konstantinb Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Nov 13, 2026

Quality Gate passed

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

konstantinb commented May 21, 2026 •

edited

Loading

konstantinb Nov 5, 2026 •

edited

Loading