Copilot commented on code in PR #17186:
URL: https://github.com/apache/pinot/pull/17186#discussion_r2517235151
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/stats/NoDictColumnStatisticsCollector.java:
##########
@@ -223,10 +242,35 @@ public void seal() {
private void updateHllPlus(Object value) {
if (value instanceof BigDecimal) {
- // Canonicalize BigDecimal as string to avoid scale-related equality
issues
+ // Canonicalize BigDecimal as string to avoid scale-related equality
issues:
+ // BigDecimals with different scales (e.g., 1.0 vs 1.00) are not equal
by default,
+ // but their string representations normalize the value for cardinality
tracking.
_hllPlus.offer(((BigDecimal) value).toString());
} else {
_hllPlus.offer(value);
}
}
+
+ private void trackExactUnique(Object value) {
+ Set<Object> exact = _exactUniquesRef.get(); // local snapshot to avoid
check-then-act race
+ if (exact == null) {
+ return;
+ }
Review Comment:
After checking `exact != null`, another thread could execute
`compareAndSet(exact, null)` on line 273 before `exact.add(key)` on line 270,
causing a NullPointerException. While the local snapshot prevents the
check-then-act race on the reference itself, it doesn't protect against the set
being nulled out by another thread's `compareAndSet` after this method captures
the reference. Consider adding a try-catch around `exact.add(key)` to handle
concurrent nullification gracefully, or document that this is acceptable
behavior (failed adds after threshold are benign).
--
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]
