Jackie-Jiang commented on code in PR #19227:
URL: https://github.com/apache/pinot/pull/19227#discussion_r3770528976
##########
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionNullContractTest.java:
##########
@@ -253,7 +253,11 @@ public void
testValueFunctionsReturnNullWhenNothingAggregated() {
AggregationFunctionType.SUMLONG, AggregationFunctionType.SUMPRECISION,
AggregationFunctionType.FIRSTWITHTIME,
AggregationFunctionType.LASTWITHTIME, AggregationFunctionType.ARRAYAGG,
AggregationFunctionType.LISTAGG,
// Given the option so they can skip null rows; a row counts only when
both input columns are non-null
- AggregationFunctionType.COVARPOP, AggregationFunctionType.COVARSAMP
+ AggregationFunctionType.COVARPOP, AggregationFunctionType.COVARSAMP,
+ // Given the option so they can skip null rows. These two were in this
set once before, on the strength of an
+ // identity comparison that reported every serializer-valued function as
honouring it; they belong here now
+ // because they genuinely do.
+ AggregationFunctionType.FREQUENTSTRINGSSKETCH,
AggregationFunctionType.FREQUENTLONGSSKETCH
Review Comment:
Agreed, and both gaps are now covered. `FrequentSketchNullHandlingTest` is
new and drives what the census cannot: the `BYTES` path with real serialized
sketches and a null bitmap, the group-by path, the all-null case, the
zero-length block, and both option settings.
You are right about why the census does not reach them — it stops at the
first usable shape and only calls `aggregate`, so `STRING` and `LONG` were the
only paths it ever exercised here. The new test says so in its Javadoc, so the
next reader does not mistake a green contract suite for coverage of this family.
--
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]
