somandal commented on code in PR #10649:
URL: https://github.com/apache/pinot/pull/10649#discussion_r1171902369
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileTDigestAggregationFunction.java:
##########
@@ -34,25 +34,40 @@
/**
* TDigest based Percentile aggregation function.
+ *
+ * TODO: Decided not to support custom compression for version 0 queries as it
seems to be the older syntax and requires
+ * extra handling for two argument PERCENTILE functions to assess if v0
or v1. This can be revisited later if the
+ * need arises
*/
public class PercentileTDigestAggregationFunction extends
BaseSingleInputAggregationFunction<TDigest, Double> {
public static final int DEFAULT_TDIGEST_COMPRESSION = 100;
- //version 0 functions specified in the of form
PERCENTILETDIGEST<2-digits>(column)
- //version 1 functions of form PERCENTILETDIGEST(column,
<2-digits>.<16-digits>)
+ // version 0 functions specified in the of form
PERCENTILETDIGEST<2-digits>(column). Uses default compression of 100
+ // version 1 functions of form PERCENTILETDIGEST(column,
<2-digits>.<16-digits>, <2-digits>.<16-digits> [optional])
Review Comment:
the TDigest API takes the compressionFactor as a double, but I checked the
code and found it uses Math.ceil() to round up :) changed this to int
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileTDigestAggregationFunction.java:
##########
@@ -34,25 +34,40 @@
/**
* TDigest based Percentile aggregation function.
+ *
+ * TODO: Decided not to support custom compression for version 0 queries as it
seems to be the older syntax and requires
+ * extra handling for two argument PERCENTILE functions to assess if v0
or v1. This can be revisited later if the
+ * need arises
*/
public class PercentileTDigestAggregationFunction extends
BaseSingleInputAggregationFunction<TDigest, Double> {
public static final int DEFAULT_TDIGEST_COMPRESSION = 100;
- //version 0 functions specified in the of form
PERCENTILETDIGEST<2-digits>(column)
- //version 1 functions of form PERCENTILETDIGEST(column,
<2-digits>.<16-digits>)
+ // version 0 functions specified in the of form
PERCENTILETDIGEST<2-digits>(column). Uses default compression of 100
+ // version 1 functions of form PERCENTILETDIGEST(column,
<2-digits>.<16-digits>, <2-digits>.<16-digits> [optional])
Review Comment:
the TDigest API takes the compressionFactor as a double, but I checked the
code and found it uses Math.ceil() to round up :) changed this to int
--
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]
