Hi Konstantin, Thanks for your proposal & contribution! As I have some dedicated time for code reviews in the current iteration, I can probably have a look.
Regards, Okumin On Tue, Oct 27, 2026 at 2:58 Konstantin Bereznyakov <[email protected]> wrote: > Hi All, > > Following up on my review request for HIVE-29625: > https://github.com/apache/hive/pull/6505 The PR distinguishes unknown > NDV (-1) from zero distinct non-NULL values, following the existing > negative-means-unknown convention for the other column statistics. > This takes up the "ideal" direction suggested during the review of PR > 6359, without introducing another ColStatistics flag. > > To make the review easier, the description now includes a reading > order, the propagation rules, and the testing details. The production > changes are across eight files; most of the diff is tests, and only > two existing query-test goldens change. CI passed on the current head. > > Would someone familiar with optimizer statistics have time to take a > look? A first pass on the convention and its propagation would be very > helpful, and I'm happy to address any feedback. > > Thanks, > Konstantin > > > On Tue, Aug 11, 2026 at 4:59 PM Konstantin Bereznyakov > <[email protected]> wrote: > > > > Hi All, > > > > I am looking for a committer to review > > PR: https://github.com/apache/hive/pull/6505 > > JIRA: https://issues.apache.org/jira/browse/HIVE-29625 > > > > ColStatistics.countDistinct currently overloads 0: a genuinely > > verified zero (all-NULL column, empty table) is indistinguishable > > from "NDV was never computed", which leaks through as the Thrift > > default. Consumers therefore apply the same fallback heuristics > > (numRows/2, factor *= 0.5, ...) to both cases - wrong for verified > > zero, and masking absent information for unknown. The PR establishes > > that a negative countDistinct means "unknown", parallel to the > > historical "negative = unknown" convention for > > numNulls/numTrues/numFalses, which HIVE-29438 reinforced. It follows > > the direction suggested during review of PR 6359: > > https://github.com/apache/hive/pull/6359#discussion_r2994697977 > > as "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." since it ultimately > > appears as the most pragmatic approach to solve a large class of NDV > > problems. > > > > The PR is not a draft, precommit and SonarCloud are green on the > > current head, and it is 8 production files plus 13 test files with > > no open review threads. It generalizes and supersedes my earlier > > point fix in PR 6418 (HIVE-29556), which I will close when/if this > > direction is confirmed. > > > > Happy to address any feedback. > > > > Thanks, > > Konstantin >
