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]

Reply via email to