konstantinb commented on PR #6505:
URL: https://github.com/apache/hive/pull/6505#issuecomment-5094525001
Hi @zabetak — gentle nudge on this one, with a recap since it's had some
hardening since you
last looked.
First, closing the two open questions I raised above so nothing dangles
before a review:
- **const-null NDV (0 vs 1):** resolved by this PR — under the `-1`
convention,
`buildColStatForConstant` assigning NDV `0` to a NULL literal is now
*correct* (zero non-null
distinct values, `numNulls` carries the rest). The metastore persisting
`1` for an all-NULL
column is a separate producer (the HLL/FMSketch clamp); I'll file that as
a follow-up. No
change needed here.
- **Inferring NDV from numNulls:** I'd suggest *not* pursuing it (prototype
in konstantinb#1
for the record) — the audience is narrow and, since `numRows` and
`numNulls` come from
different stats, drift could fabricate a false *verified* zero, which is
the exact confusion
this PR removes. Happy to file a low-priority follow-up if you'd rather
keep it.
Neither blocks the core `-1`-as-unknown change, so it stands on its own now.
Since the last ping: added regression coverage for the one case with real
plan impact — a join
with mixed known/unknown key NDVs (binary key vs a `cast` expression), where
without the fix
the estimate degrades to a cross product; `join_ndv_unknown_mixed*.q` pin it
back to parity.
CI is green, the SonarCloud gate passes, and the description now spells out
the design and the
deliberate scope boundaries.
It's still the foundational piece — #6418 and #6359 stay in draft pending
the direction this
establishes, so even a directional pass on the `-1` sentinel approach would
unblock the rest.
No rush if you're swamped.
--
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]