You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/07/29 07:38:28 UTC
[GitHub] [pinot] lakshmanan-v opened a new pull request #7226: Add functions to return raw results for Percentile TDigest and Est
lakshmanan-v opened a new pull request #7226:
URL: https://github.com/apache/pinot/pull/7226
## Description
Adding aggregate functions to return serialized / raw values of PercentileEst (QuantileDigest) and PercentileTDigest (TDigest) data structures.
-->
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [X ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
Will raise an another PR to update the documentation once this PR is approved and the changes are finalized.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r692639683
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,141 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawEstAggregationFunction} returns the serialized {@code QuantileDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
Review comment:
updated.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r687275500
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/ExpectedQueryResult.java
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+public class ExpectedQueryResult<T> {
+ private long numDocsScanned;
Review comment:
Please prefix with `_` for the member variables
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/ExpectedQueryResult.java
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+public class ExpectedQueryResult<T> {
+ private long numDocsScanned;
+ private long numEntriesScannedInFilter;
+ private long numEntriesScannedPostFilter;
+ private long numTotalDocs;
+ private T[] results;
+
+ public ExpectedQueryResult(long numDocsScanned, long numEntriesScannedInFilter, long numEntriesScannedPostFilter,
+ long numTotalDocs, T[] results) {
+ this.numDocsScanned = numDocsScanned;
Review comment:
Remove `this.`
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r685469674
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
Sounds good. Do we have a client side library ?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r695098753
##########
File path: pinot-common/src/test/java/org/apache/pinot/common/function/AggregationFunctionTypeTest.java
##########
@@ -48,6 +48,12 @@ public void testGetAggregationFunctionType() {
AggregationFunctionType.PERCENTILEEST);
Assert.assertEquals(AggregationFunctionType.getAggregationFunctionType("PeRcEnTiLeTdIgEsT99"),
AggregationFunctionType.PERCENTILETDIGEST);
+ Assert
Review comment:
The code format seems incorrect
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/SerializedBytesQueriesTest.java
##########
@@ -94,7 +94,7 @@
// Use non-default compression
private static final double PERCENTILE_TDIGEST_COMPRESSION = 200;
// Allow 5% quantile error due to the randomness of TDigest merge
- private static final double PERCENTILE_TDIGEST_DELTA = 0.05 * Integer.MAX_VALUE;
+ public static final double PERCENTILE_TDIGEST_DELTA = 0.05 * Integer.MAX_VALUE;
Review comment:
Don't expose this value outside of this test
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedQuantileDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of QuantileDigest. Compares QuantileDigest for a specific percentile value.
+ */
+public class SerializedQuantileDigest implements Comparable<SerializedQuantileDigest> {
+ private final double _percentile;
+ private final QuantileDigest _quantileDigest;
+
+ public SerializedQuantileDigest(QuantileDigest quantileDigest, double percentile) {
+ _quantileDigest = quantileDigest;
+ _percentile = percentile;
+ }
+
+ @Override
+ public int compareTo(SerializedQuantileDigest other) {
+ checkArgument(other._percentile == _percentile, "Percentile number doesn't match!");
+ return Double.compare(_quantileDigest.getQuantile(_percentile / 100.0),
+ other._quantileDigest.getQuantile(_percentile / 100.0));
+ }
+
+ @Override
+ public String toString() {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(_quantileDigest));
+ }
+}
Review comment:
(nit) new line
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedTDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import com.tdunning.math.stats.TDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of TDigest. Compares TDigest for a specific percentile value.
+ */
+public class SerializedTDigest implements Comparable<SerializedTDigest> {
+ private final double _percentile;
+ private final TDigest _tDigest;
+
+ public SerializedTDigest(TDigest tDigest, double percentile) {
+ _tDigest = tDigest;
+ _percentile = percentile;
Review comment:
Same
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedQuantileDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of QuantileDigest. Compares QuantileDigest for a specific percentile value.
+ */
+public class SerializedQuantileDigest implements Comparable<SerializedQuantileDigest> {
+ private final double _percentile;
+ private final QuantileDigest _quantileDigest;
+
+ public SerializedQuantileDigest(QuantileDigest quantileDigest, double percentile) {
+ _quantileDigest = quantileDigest;
+ _percentile = percentile;
Review comment:
Directly save `percentile / 100.0` to avoid repeating computation in `compareTo`
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedQuantileDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of QuantileDigest. Compares QuantileDigest for a specific percentile value.
+ */
+public class SerializedQuantileDigest implements Comparable<SerializedQuantileDigest> {
+ private final double _percentile;
+ private final QuantileDigest _quantileDigest;
+
+ public SerializedQuantileDigest(QuantileDigest quantileDigest, double percentile) {
+ _quantileDigest = quantileDigest;
+ _percentile = percentile;
+ }
+
+ @Override
+ public int compareTo(SerializedQuantileDigest other) {
+ checkArgument(other._percentile == _percentile, "Percentile number doesn't match!");
+ return Double.compare(_quantileDigest.getQuantile(_percentile / 100.0),
Review comment:
Use `Long.compare`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawTDigestAggregationFunction.java
##########
@@ -0,0 +1,142 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import com.tdunning.math.stats.TDigest;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawTDigestAggregationFunction} returns the serialized {@code TDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawTDigestAggregationFunction extends BaseSingleInputAggregationFunction<TDigest, String> {
+ private final PercentileTDigestAggregationFunction _percentileRawTDigestAggregationFunction;
+
+ public PercentileRawTDigestAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileTDigestAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawTDigestAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileTDigestAggregationFunction(expressionContext, percentile));
+ }
+
+ protected PercentileRawTDigestAggregationFunction(ExpressionContext expression,
+ PercentileTDigestAggregationFunction percentileRawTDigestAggregationFunction) {
Review comment:
Still not correct
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r679296306
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,210 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
Review comment:
Most of the tests in this file use this skeleton. There was already a refactor to use testInterSegmentAggregationResult() which simplified this a bit. We should be able to refactor it further by passing the query, expected values etc.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7226:
URL: https://github.com/apache/pinot/pull/7226
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r692639552
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/ExpectedQueryResult.java
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+public class ExpectedQueryResult<T> {
+ private long numDocsScanned;
Review comment:
updated.
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/ExpectedQueryResult.java
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+public class ExpectedQueryResult<T> {
+ private long numDocsScanned;
+ private long numEntriesScannedInFilter;
+ private long numEntriesScannedPostFilter;
+ private long numTotalDocs;
+ private T[] results;
+
+ public ExpectedQueryResult(long numDocsScanned, long numEntriesScannedInFilter, long numEntriesScannedPostFilter,
+ long numTotalDocs, T[] results) {
+ this.numDocsScanned = numDocsScanned;
Review comment:
updated.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawTDigestAggregationFunction.java
##########
@@ -0,0 +1,142 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import com.tdunning.math.stats.TDigest;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawTDigestAggregationFunction} returns the serialized {@code TDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawTDigestAggregationFunction extends BaseSingleInputAggregationFunction<TDigest, String> {
+ private final PercentileTDigestAggregationFunction _percentileRawTDigestAggregationFunction;
Review comment:
updated.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r686134630
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
Will look into it and add some compress logic if needed after this PR is landed.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r681413735
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,210 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
Review comment:
Refactored a bit to avoid duplicate code.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r692639060
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawTDigestAggregationFunction.java
##########
@@ -0,0 +1,142 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import com.tdunning.math.stats.TDigest;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawTDigestAggregationFunction} returns the serialized {@code TDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawTDigestAggregationFunction extends BaseSingleInputAggregationFunction<TDigest, String> {
+ private final PercentileTDigestAggregationFunction _percentileRawTDigestAggregationFunction;
+
+ public PercentileRawTDigestAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileTDigestAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawTDigestAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileTDigestAggregationFunction(expressionContext, percentile));
+ }
+
+ protected PercentileRawTDigestAggregationFunction(ExpressionContext expression,
+ PercentileTDigestAggregationFunction percentileRawTDigestAggregationFunction) {
Review comment:
Updated.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r679198631
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
Review comment:
Please add java docs for all public class/methods?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
Just talking to myself here - we have used this utility function in several places, and I can never remember its name (including ByteUtils). Perhaps we should make a Pinot Utils wrapper and use that. (Not a blocker for this PR).
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,210 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
Review comment:
We should be able to refactor these tests so that we don't have to write the same code for each of the percentile value test right?
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationSingleValueQueriesTest.java
##########
@@ -412,6 +412,217 @@ public void testPercentileEst99() {
new String[]{"2146232405", "999309554"});
}
+ @Test
+ public void testPercentileRawEst50() {
Review comment:
same here.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
Review comment:
Add explicit access modifier?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r686356725
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
Review comment:
Done.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (372a27a) into [master](https://codecov.io/gh/apache/pinot/commit/5af607813ba2cdb63694592cb3507600d98f75d8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5af6078) will **decrease** coverage by `4.42%`.
> The diff coverage is `88.07%`.
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 73.50% 69.08% -4.43%
- Complexity 92 3183 +3091
============================================
Files 1508 1115 -393
Lines 73951 52426 -21525
Branches 10685 7824 -2861
============================================
- Hits 54356 36217 -18139
+ Misses 16027 13638 -2389
+ Partials 3568 2571 -997
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `?` | |
| unittests1 | `69.08% <88.07%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...unction/PercentileRawEstMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0TVZBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [...ion/PercentileRawTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `60.00% <60.00%> (ø)` | |
| [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `86.73% <75.00%> (-1.73%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `97.14% <83.33%> (-2.86%)` | :arrow_down: |
| [.../function/PercentileRawEstAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `96.96% <96.96%> (ø)` | |
| [...ction/PercentileRawTDigestAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `97.05% <97.05%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/spi/stream/PartitionGroupMetadata.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1BhcnRpdGlvbkdyb3VwTWV0YWRhdGEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [743 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5af6078...372a27a](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895726791
plz test
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r687015521
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
Review comment:
Done.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (372a27a) into [master](https://codecov.io/gh/apache/pinot/commit/5af607813ba2cdb63694592cb3507600d98f75d8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5af6078) will **decrease** coverage by `2.23%`.
> The diff coverage is `88.07%`.
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 73.50% 71.26% -2.24%
- Complexity 92 3359 +3267
============================================
Files 1508 1512 +4
Lines 73951 74116 +165
Branches 10685 10709 +24
============================================
- Hits 54356 52819 -1537
- Misses 16027 17710 +1683
- Partials 3568 3587 +19
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| integration1 | `29.79% <0.00%> (?)` | |
| integration2 | `28.87% <0.00%> (?)` | |
| unittests | `?` | |
| unittests1 | `69.08% <88.07%> (?)` | |
| unittests2 | `14.40% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...unction/PercentileRawEstMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0TVZBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [...ion/PercentileRawTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `60.00% <60.00%> (ø)` | |
| [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `86.73% <75.00%> (-1.73%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `97.14% <83.33%> (-2.86%)` | :arrow_down: |
| [.../function/PercentileRawEstAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `96.96% <96.96%> (ø)` | |
| [...ction/PercentileRawTDigestAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `97.05% <97.05%> (ø)` | |
| [...g/apache/pinot/server/api/resources/ErrorInfo.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9FcnJvckluZm8uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/spi/stream/PartitionGroupMetadata.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1BhcnRpdGlvbkdyb3VwTWV0YWRhdGEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/common/exception/TableNotFoundException.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1RhYmxlTm90Rm91bmRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [211 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5af6078...372a27a](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r685648784
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
We have the `pinot-client` library.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (09d54da) into [master](https://codecov.io/gh/apache/pinot/commit/5af607813ba2cdb63694592cb3507600d98f75d8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5af6078) will **increase** coverage by `0.07%`.
> The diff coverage is `84.63%`.
> :exclamation: Current head 09d54da differs from pull request most recent head 417946d. Consider uploading reports for the commit 417946d to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
+ Coverage 73.50% 73.58% +0.07%
Complexity 92 92
============================================
Files 1508 1512 +4
Lines 73951 74116 +165
Branches 10685 10709 +24
============================================
+ Hits 54356 54536 +180
+ Misses 16027 16006 -21
- Partials 3568 3574 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.79% <26.56%> (-0.04%)` | :arrow_down: |
| unittests | `65.37% <82.81%> (+0.08%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `19.47% <ø> (+0.20%)` | :arrow_up: |
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...l/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `65.00% <ø> (-0.58%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `81.45% <0.00%> (-0.67%)` | :arrow_down: |
| [...egment/spi/index/metadata/SegmentMetadataImpl.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L21ldGFkYXRhL1NlZ21lbnRNZXRhZGF0YUltcGwuamF2YQ==) | `74.11% <0.00%> (-0.59%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/TableConfig.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlQ29uZmlnLmphdmE=) | `81.69% <0.00%> (-1.17%)` | :arrow_down: |
| [...inot/spi/ingestion/batch/IngestionJobLauncher.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvaW5nZXN0aW9uL2JhdGNoL0luZ2VzdGlvbkpvYkxhdW5jaGVyLmphdmE=) | `19.04% <0.00%> (-3.81%)` | :arrow_down: |
| [...ingestion/batch/spec/SegmentGenerationJobSpec.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvaW5nZXN0aW9uL2JhdGNoL3NwZWMvU2VnbWVudEdlbmVyYXRpb25Kb2JTcGVjLmphdmE=) | `100.00% <ø> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `35.00% <ø> (ø)` | |
| [...unction/PercentileRawEstMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0TVZBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| ... and [65 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5af6078...417946d](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r687267355
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,119 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
+ testPercentileEstAggregationResult(50);
+ }
+
+ @Test
+ public void testPercentileRawEst90MV() {
+ testPercentileEstAggregationResult(90);
+ }
+
+ @Test
+ public void testPercentileRawEst95MV() {
+ testPercentileEstAggregationResult(95);
+ }
+
+ @Test
+ public void testPercentileRawEst99MV() {
+ testPercentileEstAggregationResult(99);
+ }
+
+ private void testPercentileEstAggregationResult(int percentile) {
+
+ Function<Serializable, String> quantileExtractor = value -> String.valueOf(ObjectSerDeUtils.QUANTILE_DIGEST_SER_DE.deserialize(BytesUtils.toBytes((String) value))
+ .getQuantile((double) percentile / 100.0));
+
+ ExpectedQueryResult<Long> expectedQueryResultsBasic =
Review comment:
I was thinking we should modify the tests as follows:
1. Run a query with non raw version - get the result
2. Run the same query with raw version - deserialize - get the result
3. Assert that results from 1 and 2 should be exact match.
This will take away any hardcoding as well as needing to check if the results is within a % margin.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aa833e9) into [master](https://codecov.io/gh/apache/pinot/commit/767a149773a27fb38fadfd1448fd6162dff510ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (767a149) will **decrease** coverage by `2.09%`.
> The diff coverage is `87.90%`.
> :exclamation: Current head aa833e9 differs from pull request most recent head 907783f. Consider uploading reports for the commit 907783f to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 71.63% 69.53% -2.10%
+ Complexity 3297 3216 -81
============================================
Files 1509 1121 -388
Lines 74868 52926 -21942
Branches 10887 7958 -2929
============================================
- Hits 53629 36803 -16826
+ Misses 17608 13512 -4096
+ Partials 3631 2611 -1020
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.53% <87.90%> (+0.10%)` | :arrow_up: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...unction/PercentileRawEstMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0TVZBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [...ion/PercentileRawTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `60.00% <60.00%> (ø)` | |
| [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `86.86% <75.00%> (-1.74%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `97.18% <83.33%> (-2.82%)` | :arrow_down: |
| [.../segment/local/customobject/SerializedTDigest.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvU2VyaWFsaXplZFREaWdlc3QuamF2YQ==) | `85.71% <85.71%> (ø)` | |
| [...t/local/customobject/SerializedQuantileDigest.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvU2VyaWFsaXplZFF1YW50aWxlRGlnZXN0LmphdmE=) | `87.50% <87.50%> (ø)` | |
| [.../function/PercentileRawEstAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `96.96% <96.96%> (ø)` | |
| [...ction/PercentileRawTDigestAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `97.05% <97.05%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [617 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [767a149...907783f](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r692639497
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,119 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
+ testPercentileEstAggregationResult(50);
+ }
+
+ @Test
+ public void testPercentileRawEst90MV() {
+ testPercentileEstAggregationResult(90);
+ }
+
+ @Test
+ public void testPercentileRawEst95MV() {
+ testPercentileEstAggregationResult(95);
+ }
+
+ @Test
+ public void testPercentileRawEst99MV() {
+ testPercentileEstAggregationResult(99);
+ }
+
+ private void testPercentileEstAggregationResult(int percentile) {
+
+ Function<Serializable, String> quantileExtractor = value -> String.valueOf(ObjectSerDeUtils.QUANTILE_DIGEST_SER_DE.deserialize(BytesUtils.toBytes((String) value))
+ .getQuantile((double) percentile / 100.0));
+
+ ExpectedQueryResult<Long> expectedQueryResultsBasic =
Review comment:
Added hard coded checks for group by queries and comparison against original queries for basic query and query with filter.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mayankshriv commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r683866618
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationSingleValueQueriesTest.java
##########
@@ -412,6 +412,186 @@ public void testPercentileEst99() {
new String[]{"2146232405", "999309554"});
}
+ @Test
+ public void testPercentileRawEst50() {
+ ExpectedQueryResult expectedQueryResultsBasic =
+ new ExpectedQueryResult(120000L, 0L, 240000L, 120000L, new String[]{"1107310944", "1082130431"});
Review comment:
These are probably brittle tests since we are hardcoding the expected results? We should probably test that results are within a certain error margin of the accurate percentile?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
We can do this outside this PR. Perhaps provide a wrapper to encode/decode these bytes, and share it in the client side library. Once we have a wrapper, we can do all sorts of things like compression etc. One reason why we did String was so UI can show it without breaking. Would have to evaluate that as well.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aa833e9) into [master](https://codecov.io/gh/apache/pinot/commit/767a149773a27fb38fadfd1448fd6162dff510ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (767a149) will **decrease** coverage by `1.19%`.
> The diff coverage is `87.90%`.
> :exclamation: Current head aa833e9 differs from pull request most recent head 907783f. Consider uploading reports for the commit 907783f to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 71.63% 70.43% -1.20%
- Complexity 3297 3308 +11
============================================
Files 1509 1515 +6
Lines 74868 74992 +124
Branches 10887 10903 +16
============================================
- Hits 53629 52822 -807
- Misses 17608 18552 +944
+ Partials 3631 3618 -13
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `29.03% <0.00%> (-0.15%)` | :arrow_down: |
| unittests1 | `69.53% <87.90%> (+0.10%)` | :arrow_up: |
| unittests2 | `14.46% <0.00%> (-0.03%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...unction/PercentileRawEstMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0TVZBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [...ion/PercentileRawTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `60.00% <60.00%> (ø)` | |
| [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `86.86% <75.00%> (-1.74%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `97.18% <83.33%> (-2.82%)` | :arrow_down: |
| [.../segment/local/customobject/SerializedTDigest.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvU2VyaWFsaXplZFREaWdlc3QuamF2YQ==) | `85.71% <85.71%> (ø)` | |
| [...t/local/customobject/SerializedQuantileDigest.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvU2VyaWFsaXplZFF1YW50aWxlRGlnZXN0LmphdmE=) | `87.50% <87.50%> (ø)` | |
| [.../function/PercentileRawEstAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `96.96% <96.96%> (ø)` | |
| [...ction/PercentileRawTDigestAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `97.05% <97.05%> (ø)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [117 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [767a149...907783f](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r695138853
##########
File path: pinot-common/src/test/java/org/apache/pinot/common/function/AggregationFunctionTypeTest.java
##########
@@ -48,6 +48,12 @@ public void testGetAggregationFunctionType() {
AggregationFunctionType.PERCENTILEEST);
Assert.assertEquals(AggregationFunctionType.getAggregationFunctionType("PeRcEnTiLeTdIgEsT99"),
AggregationFunctionType.PERCENTILETDIGEST);
+ Assert
Review comment:
Updated.
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedQuantileDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of QuantileDigest. Compares QuantileDigest for a specific percentile value.
+ */
+public class SerializedQuantileDigest implements Comparable<SerializedQuantileDigest> {
+ private final double _percentile;
+ private final QuantileDigest _quantileDigest;
+
+ public SerializedQuantileDigest(QuantileDigest quantileDigest, double percentile) {
+ _quantileDigest = quantileDigest;
+ _percentile = percentile;
+ }
+
+ @Override
+ public int compareTo(SerializedQuantileDigest other) {
+ checkArgument(other._percentile == _percentile, "Percentile number doesn't match!");
+ return Double.compare(_quantileDigest.getQuantile(_percentile / 100.0),
Review comment:
Updated.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r686356853
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,210 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
Review comment:
Done.
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationSingleValueQueriesTest.java
##########
@@ -412,6 +412,217 @@ public void testPercentileEst99() {
new String[]{"2146232405", "999309554"});
}
+ @Test
+ public void testPercentileRawEst50() {
Review comment:
Done.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (09d54da) into [master](https://codecov.io/gh/apache/pinot/commit/5af607813ba2cdb63694592cb3507600d98f75d8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5af6078) will **decrease** coverage by `8.13%`.
> The diff coverage is `82.81%`.
> :exclamation: Current head 09d54da differs from pull request most recent head 417946d. Consider uploading reports for the commit 417946d to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 73.50% 65.37% -8.14%
Complexity 92 92
============================================
Files 1508 1512 +4
Lines 73951 74116 +165
Branches 10685 10709 +24
============================================
- Hits 54356 48451 -5905
- Misses 16027 22245 +6218
+ Partials 3568 3420 -148
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.37% <82.81%> (+0.08%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `3.15% <ø> (-16.12%)` | :arrow_down: |
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `10.61% <0.00%> (-56.47%)` | :arrow_down: |
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...l/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `61.66% <ø> (-3.91%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `78.22% <0.00%> (-3.89%)` | :arrow_down: |
| [...egment/spi/index/metadata/SegmentMetadataImpl.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L21ldGFkYXRhL1NlZ21lbnRNZXRhZGF0YUltcGwuamF2YQ==) | `74.11% <0.00%> (-0.59%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/TableConfig.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlQ29uZmlnLmphdmE=) | `81.69% <0.00%> (-1.17%)` | :arrow_down: |
| [...inot/spi/ingestion/batch/IngestionJobLauncher.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvaW5nZXN0aW9uL2JhdGNoL0luZ2VzdGlvbkpvYkxhdW5jaGVyLmphdmE=) | `19.04% <0.00%> (-3.81%)` | :arrow_down: |
| [...ingestion/batch/spec/SegmentGenerationJobSpec.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvaW5nZXN0aW9uL2JhdGNoL3NwZWMvU2VnbWVudEdlbmVyYXRpb25Kb2JTcGVjLmphdmE=) | `100.00% <ø> (ø)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `35.00% <ø> (ø)` | |
| ... and [393 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5af6078...417946d](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r695138429
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/SerializedBytesQueriesTest.java
##########
@@ -94,7 +94,7 @@
// Use non-default compression
private static final double PERCENTILE_TDIGEST_COMPRESSION = 200;
// Allow 5% quantile error due to the randomness of TDigest merge
- private static final double PERCENTILE_TDIGEST_DELTA = 0.05 * Integer.MAX_VALUE;
+ public static final double PERCENTILE_TDIGEST_DELTA = 0.05 * Integer.MAX_VALUE;
Review comment:
Updated.
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedQuantileDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of QuantileDigest. Compares QuantileDigest for a specific percentile value.
+ */
+public class SerializedQuantileDigest implements Comparable<SerializedQuantileDigest> {
+ private final double _percentile;
+ private final QuantileDigest _quantileDigest;
+
+ public SerializedQuantileDigest(QuantileDigest quantileDigest, double percentile) {
+ _quantileDigest = quantileDigest;
+ _percentile = percentile;
+ }
+
+ @Override
+ public int compareTo(SerializedQuantileDigest other) {
+ checkArgument(other._percentile == _percentile, "Percentile number doesn't match!");
+ return Double.compare(_quantileDigest.getQuantile(_percentile / 100.0),
+ other._quantileDigest.getQuantile(_percentile / 100.0));
+ }
+
+ @Override
+ public String toString() {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(_quantileDigest));
+ }
+}
Review comment:
Fixed.
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedQuantileDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of QuantileDigest. Compares QuantileDigest for a specific percentile value.
+ */
+public class SerializedQuantileDigest implements Comparable<SerializedQuantileDigest> {
+ private final double _percentile;
+ private final QuantileDigest _quantileDigest;
+
+ public SerializedQuantileDigest(QuantileDigest quantileDigest, double percentile) {
+ _quantileDigest = quantileDigest;
+ _percentile = percentile;
Review comment:
Done.
##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedTDigest.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.segment.local.customobject;
+
+import com.tdunning.math.stats.TDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Serialized and comparable version of TDigest. Compares TDigest for a specific percentile value.
+ */
+public class SerializedTDigest implements Comparable<SerializedTDigest> {
+ private final double _percentile;
+ private final TDigest _tDigest;
+
+ public SerializedTDigest(TDigest tDigest, double percentile) {
+ _tDigest = tDigest;
+ _percentile = percentile;
Review comment:
Done.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r691677092
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueQueriesTest.java
##########
@@ -519,6 +519,119 @@ public void testPercentileEst99MV() {
.testInterSegmentAggregationResult(brokerResponse, 400000L, 0L, 800000L, 400000L, new String[]{"2147483647"});
}
+ @Test
+ public void testPercentileRawEst50MV() {
+ testPercentileEstAggregationResult(50);
+ }
+
+ @Test
+ public void testPercentileRawEst90MV() {
+ testPercentileEstAggregationResult(90);
+ }
+
+ @Test
+ public void testPercentileRawEst95MV() {
+ testPercentileEstAggregationResult(95);
+ }
+
+ @Test
+ public void testPercentileRawEst99MV() {
+ testPercentileEstAggregationResult(99);
+ }
+
+ private void testPercentileEstAggregationResult(int percentile) {
+
+ Function<Serializable, String> quantileExtractor = value -> String.valueOf(ObjectSerDeUtils.QUANTILE_DIGEST_SER_DE.deserialize(BytesUtils.toBytes((String) value))
+ .getQuantile((double) percentile / 100.0));
+
+ ExpectedQueryResult<Long> expectedQueryResultsBasic =
Review comment:
The result might not be deterministic because of the merging order of the segments though
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aa833e9) into [master](https://codecov.io/gh/apache/pinot/commit/767a149773a27fb38fadfd1448fd6162dff510ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (767a149) will **decrease** coverage by `6.80%`.
> The diff coverage is `87.90%`.
> :exclamation: Current head aa833e9 differs from pull request most recent head 907783f. Consider uploading reports for the commit 907783f to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 71.63% 64.82% -6.81%
- Complexity 3297 3308 +11
============================================
Files 1509 1469 -40
Lines 74868 73067 -1801
Branches 10887 10697 -190
============================================
- Hits 53629 47369 -6260
- Misses 17608 22322 +4714
+ Partials 3631 3376 -255
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.53% <87.90%> (+0.10%)` | :arrow_up: |
| unittests2 | `14.46% <0.00%> (-0.03%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...unction/PercentileRawEstMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0TVZBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [...ion/PercentileRawTDigestMVAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdE1WQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `60.00% <60.00%> (ø)` | |
| [...gregation/function/AggregationFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `86.86% <75.00%> (-1.74%)` | :arrow_down: |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `97.18% <83.33%> (-2.82%)` | :arrow_down: |
| [.../segment/local/customobject/SerializedTDigest.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvU2VyaWFsaXplZFREaWdlc3QuamF2YQ==) | `85.71% <85.71%> (ø)` | |
| [...t/local/customobject/SerializedQuantileDigest.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvU2VyaWFsaXplZFF1YW50aWxlRGlnZXN0LmphdmE=) | `87.50% <87.50%> (ø)` | |
| [.../function/PercentileRawEstAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3RXN0QWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `96.96% <96.96%> (ø)` | |
| [...ction/PercentileRawTDigestAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9QZXJjZW50aWxlUmF3VERpZ2VzdEFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `97.05% <97.05%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [378 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [767a149...907783f](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r679301301
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
@mayankshriv Some of these serialized hex strings are little longer (about 3K bytes). Thinking of compressing this using [Snappy](https://github.com/xerial/snappy-java) or [LZ4](https://github.com/lz4/lz4-java). Did a quick test using LZ4 and got the following results. Pinot seems to [use](https://github.com/apache/pinot/blob/b826f2f588bc8d0d05ef466468c2317119ce46cc/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/compression/LZ4Compressor.java) LZ4 to compress data in segments already.
Original Size = 3904; Compressed size = 1599; Compression rate = 59.04200819672132
` LZ4Factory factory = LZ4Factory.fastestInstance();
LZ4Compressor compressor = factory.highCompressor(17);
int maxCompressedLength = compressor.maxCompressedLength(decompressedLength);
byte[] compressed = new byte[maxCompressedLength];
int compressedSize = compressor.compress(data, 0, decompressedLength, compressed, 0, maxCompressedLength);
double ratio = (1 - compressedSize / (double) originalSize) * 100.0;`
Was wondering if we should convert it back to base64 string or just return byte array (lz4 produces byte array). Let me know if you have any other ideas around compressing the results.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r685469208
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationSingleValueQueriesTest.java
##########
@@ -412,6 +412,186 @@ public void testPercentileEst99() {
new String[]{"2146232405", "999309554"});
}
+ @Test
+ public void testPercentileRawEst50() {
+ ExpectedQueryResult expectedQueryResultsBasic =
+ new ExpectedQueryResult(120000L, 0L, 240000L, 120000L, new String[]{"1107310944", "1082130431"});
Review comment:
Makes sense. Updated.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] lakshmanan-v commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
lakshmanan-v commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r679301301
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,137 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawEstAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileEstAggregationFunction(expressionContext, percentile));
+ }
+
+ PercentileRawEstAggregationFunction(ExpressionContext expression,
+ PercentileEstAggregationFunction percentileRawEstAggregationFunction) {
+ super(expression);
+ _percentileRawEstAggregationFunction = percentileRawEstAggregationFunction;
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.PERCENTILERAWEST;
+ }
+
+ @Override
+ public String getColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName();
+
+ return version == 0 ? type + (int) percentile + "_" + _expression : type + percentile + "_" + _expression;
+ }
+
+ @Override
+ public String getResultColumnName() {
+ final double percentile = _percentileRawEstAggregationFunction._percentile;
+ final int version = _percentileRawEstAggregationFunction._version;
+ final String type = getType().getName().toLowerCase();
+
+ return version == 0 ? type + (int) percentile + "(" + _expression + ")"
+ : type + "(" + _expression + ", " + percentile + ")";
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return _percentileRawEstAggregationFunction.createAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return _percentileRawEstAggregationFunction.createGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregate(length, aggregationResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction.aggregateGroupBySV(length, groupKeyArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ _percentileRawEstAggregationFunction
+ .aggregateGroupByMV(length, groupKeysArray, groupByResultHolder, blockValSetMap);
+ }
+
+ @Override
+ public QuantileDigest extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return _percentileRawEstAggregationFunction.extractAggregationResult(aggregationResultHolder);
+ }
+
+ @Override
+ public QuantileDigest extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return _percentileRawEstAggregationFunction.extractGroupByResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public QuantileDigest merge(QuantileDigest intermediateResult1, QuantileDigest intermediateResult2) {
+ return _percentileRawEstAggregationFunction.merge(intermediateResult1, intermediateResult2);
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return _percentileRawEstAggregationFunction.isIntermediateResultComparable();
+ }
+
+ @Override
+ public ColumnDataType getIntermediateResultColumnType() {
+ return _percentileRawEstAggregationFunction.getIntermediateResultColumnType();
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(QuantileDigest intermediateResult) {
+ return BytesUtils.toHexString(CustomSerDeUtils.QUANTILE_DIGEST_SER_DE.serialize(intermediateResult));
Review comment:
@mayankshriv Some of these serialized hex strings are little longer (about 3K bytes). Thinking of compressing this using [Snappy](https://github.com/xerial/snappy-java) or [LZ4](https://github.com/lz4/lz4-java). Did a quick test using LZ4 and got the following results. Pinot seems to [use](https://github.com/apache/pinot/blob/b826f2f588bc8d0d05ef466468c2317119ce46cc/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/io/compression/LZ4Compressor.java) LZ4 to compress data in segments already.
**Original Size = 3904; Compressed size = 1599; Compression rate = 59.04200819672132**
```
LZ4Factory factory = LZ4Factory.fastestInstance();
LZ4Compressor compressor = factory.highCompressor(17);
int maxCompressedLength = compressor.maxCompressedLength(decompressedLength);
byte[] compressed = new byte[maxCompressedLength];
int compressedSize = compressor.compress(data, 0, decompressedLength, compressed, 0, maxCompressedLength);
double ratio = (1 - compressedSize / (double) originalSize) * 100.0;
```
Was wondering if we should convert it back to base64 string or just return byte array (lz4 produces byte array). Let me know if you have any other ideas around compressing the results.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#issuecomment-895520797
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7226](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bc371f2) into [master](https://codecov.io/gh/apache/pinot/commit/892a0c124b5c6e24bf1fe1f4f9381737d7d417cc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (892a0c1) will **decrease** coverage by `2.23%`.
> The diff coverage is `70.10%`.
> :exclamation: Current head bc371f2 differs from pull request most recent head 34bb798. Consider uploading reports for the commit 34bb798 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7226 +/- ##
============================================
- Coverage 71.57% 69.33% -2.24%
+ Complexity 3293 3195 -98
============================================
Files 1504 1114 -390
Lines 74311 52377 -21934
Branches 10798 7876 -2922
============================================
- Hits 53189 36317 -16872
+ Misses 17505 13455 -4050
+ Partials 3617 2605 -1012
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `69.33% <70.10%> (-0.02%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `84.61% <ø> (ø)` | |
| [...ent/local/data/manager/TableDataManagerConfig.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9kYXRhL21hbmFnZXIvVGFibGVEYXRhTWFuYWdlckNvbmZpZy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ot/segment/local/io/compression/LZ4Compressor.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb21wcmVzc2lvbi9MWjRDb21wcmVzc29yLmphdmE=) | `100.00% <ø> (ø)` | |
| [.../segment/local/io/compression/LZ4Decompressor.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb21wcmVzc2lvbi9MWjREZWNvbXByZXNzb3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...nt/local/io/compression/ZstandardDecompressor.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby9jb21wcmVzc2lvbi9ac3RhbmRhcmREZWNvbXByZXNzb3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...segment/local/io/util/FixedBitIntReaderWriter.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby91dGlsL0ZpeGVkQml0SW50UmVhZGVyV3JpdGVyLmphdmE=) | `85.71% <0.00%> (-0.96%)` | :arrow_down: |
| [...gment/local/io/util/FixedBitIntReaderWriterV2.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby91dGlsL0ZpeGVkQml0SW50UmVhZGVyV3JpdGVyVjIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../io/writer/impl/BaseChunkSVForwardIndexWriter.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pby93cml0ZXIvaW1wbC9CYXNlQ2h1bmtTVkZvcndhcmRJbmRleFdyaXRlci5qYXZh) | `85.71% <ø> (ø)` | |
| [...invertedindex/RealtimeLuceneIndexRefreshState.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvUmVhbHRpbWVMdWNlbmVJbmRleFJlZnJlc2hTdGF0ZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ocal/recordtransformer/ComplexTypeTransformer.java](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWNvcmR0cmFuc2Zvcm1lci9Db21wbGV4VHlwZVRyYW5zZm9ybWVyLmphdmE=) | `56.28% <ø> (ø)` | |
| ... and [749 more](https://codecov.io/gh/apache/pinot/pull/7226/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [892a0c1...34bb798](https://codecov.io/gh/apache/pinot/pull/7226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7226: Add functions to return raw results for Percentile TDigest and Est
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7226:
URL: https://github.com/apache/pinot/pull/7226#discussion_r691673175
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawTDigestAggregationFunction.java
##########
@@ -0,0 +1,142 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import com.tdunning.math.stats.TDigest;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawTDigestAggregationFunction} returns the serialized {@code TDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawTDigestAggregationFunction extends BaseSingleInputAggregationFunction<TDigest, String> {
+ private final PercentileTDigestAggregationFunction _percentileRawTDigestAggregationFunction;
Review comment:
Rename the variable
```suggestion
private final PercentileTDigestAggregationFunction _percentileTDigestAggregationFunction;
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java
##########
@@ -0,0 +1,141 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.customobject.QuantileDigest;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawEstAggregationFunction} returns the serialized {@code QuantileDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawEstAggregationFunction extends BaseSingleInputAggregationFunction<QuantileDigest, String> {
+ private final PercentileEstAggregationFunction _percentileRawEstAggregationFunction;
Review comment:
Same for this class
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawTDigestAggregationFunction.java
##########
@@ -0,0 +1,142 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+import com.tdunning.math.stats.TDigest;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.segment.local.utils.CustomSerDeUtils;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.utils.BytesUtils;
+
+
+/**
+ * The {@code PercentileRawTDigestAggregationFunction} returns the serialized {@code TDigest} data structure of the
+ * {@code PercentileEstAggregationFunction}.
+ */
+public class PercentileRawTDigestAggregationFunction extends BaseSingleInputAggregationFunction<TDigest, String> {
+ private final PercentileTDigestAggregationFunction _percentileRawTDigestAggregationFunction;
+
+ public PercentileRawTDigestAggregationFunction(ExpressionContext expressionContext, int percentile) {
+ this(expressionContext, new PercentileTDigestAggregationFunction(expressionContext, percentile));
+ }
+
+ public PercentileRawTDigestAggregationFunction(ExpressionContext expressionContext, double percentile) {
+ this(expressionContext, new PercentileTDigestAggregationFunction(expressionContext, percentile));
+ }
+
+ protected PercentileRawTDigestAggregationFunction(ExpressionContext expression,
+ PercentileTDigestAggregationFunction percentileRawTDigestAggregationFunction) {
Review comment:
Rename the parameter
```suggestion
PercentileTDigestAggregationFunction percentileTDigestAggregationFunction) {
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org