You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "davecromberge (via GitHub)" <gi...@apache.org> on 2023/10/10 14:38:12 UTC
[PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
davecromberge opened a new pull request, #11774:
URL: https://github.com/apache/pinot/pull/11774
The [CPC sketch](https://arxiv.org/abs/1708.06839) is a space-efficient cardinality estimation algorithm. More detail can be found on the Apache Datasketches website documentation: https://datasketches.apache.org
Support is added here for the following:
- ingestion via transformation function
- extracting estimates via query aggregation functions
- segment rollup aggregation
- StarTree aggregation
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on PR #11774:
URL: https://github.com/apache/pinot/pull/11774#issuecomment-1761688844
I have tested the following features on this PR:
- ingestion transformation function using baseball stats
- segment rollup aggregation with MergeRollupTask
- StarTree aggregation
- query aggregation
Accumulating results on a server into a list before merging yielded marginal performance gains at the cost of code complexity and this approach was abandoned.
Some ideas for future improvement are:
- extensible aggregation functions via plugin system
- adding sub-packages to the existing aggregation functions and factoring out common parts.
For the second point, a base class has been used in some instances and a similar approach could be used with the sketch implementations - with the exception of the Theta Sketch.
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1355088458
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -515,23 +518,23 @@ public VarianceTuple deserialize(ByteBuffer byteBuffer) {
}
};
- public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE
- = new ObjectSerDe<PinotFourthMoment>() {
- @Override
- public byte[] serialize(PinotFourthMoment value) {
- return value.serialize();
- }
+ public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE =
+ new ObjectSerDe<PinotFourthMoment>() {
+ @Override
+ public byte[] serialize(PinotFourthMoment value) {
+ return value.serialize();
+ }
- @Override
- public PinotFourthMoment deserialize(byte[] bytes) {
- return PinotFourthMoment.fromBytes(bytes);
- }
+ @Override
+ public PinotFourthMoment deserialize(byte[] bytes) {
+ return PinotFourthMoment.fromBytes(bytes);
+ }
- @Override
- public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
- return PinotFourthMoment.fromBytes(byteBuffer);
- }
- };
+ @Override
+ public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
+ return PinotFourthMoment.fromBytes(byteBuffer);
+ }
+ };
Review Comment:
I am happy to remove the additional formatting - I used the [recommended settings](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#intellij) for IntelliJ and reformatted every file I touched accordingly. This does not seem to be enforced on CI so perhaps it is unnecessary.
What do you think?
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "cbalci (via GitHub)" <gi...@apache.org>.
cbalci commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1357254877
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -515,23 +518,23 @@ public VarianceTuple deserialize(ByteBuffer byteBuffer) {
}
};
- public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE
- = new ObjectSerDe<PinotFourthMoment>() {
- @Override
- public byte[] serialize(PinotFourthMoment value) {
- return value.serialize();
- }
+ public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE =
+ new ObjectSerDe<PinotFourthMoment>() {
+ @Override
+ public byte[] serialize(PinotFourthMoment value) {
+ return value.serialize();
+ }
- @Override
- public PinotFourthMoment deserialize(byte[] bytes) {
- return PinotFourthMoment.fromBytes(bytes);
- }
+ @Override
+ public PinotFourthMoment deserialize(byte[] bytes) {
+ return PinotFourthMoment.fromBytes(bytes);
+ }
- @Override
- public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
- return PinotFourthMoment.fromBytes(byteBuffer);
- }
- };
+ @Override
+ public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
+ return PinotFourthMoment.fromBytes(byteBuffer);
+ }
+ };
Review Comment:
Not sure if there is an explicit guideline for this repo, but I think it would be better to separate functional and formatting changes into different PRs.
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "cbalci (via GitHub)" <gi...@apache.org>.
cbalci commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1353024718
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -515,23 +518,23 @@ public VarianceTuple deserialize(ByteBuffer byteBuffer) {
}
};
- public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE
- = new ObjectSerDe<PinotFourthMoment>() {
- @Override
- public byte[] serialize(PinotFourthMoment value) {
- return value.serialize();
- }
+ public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE =
+ new ObjectSerDe<PinotFourthMoment>() {
+ @Override
+ public byte[] serialize(PinotFourthMoment value) {
+ return value.serialize();
+ }
- @Override
- public PinotFourthMoment deserialize(byte[] bytes) {
- return PinotFourthMoment.fromBytes(bytes);
- }
+ @Override
+ public PinotFourthMoment deserialize(byte[] bytes) {
+ return PinotFourthMoment.fromBytes(bytes);
+ }
- @Override
- public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
- return PinotFourthMoment.fromBytes(byteBuffer);
- }
- };
+ @Override
+ public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
+ return PinotFourthMoment.fromBytes(byteBuffer);
+ }
+ };
Review Comment:
Do we need these formatting changes? They don't look relevant to the PR
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCAggregationFunction.java:
##########
@@ -0,0 +1,358 @@
+/**
+ * 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.google.common.base.Preconditions;
+import java.util.List;
+import java.util.Map;
+import org.apache.datasketches.cpc.CpcSketch;
+import org.apache.datasketches.cpc.CpcUnion;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.common.ObjectSerDeUtils;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.utils.CommonConstants;
+
+
+@SuppressWarnings({"rawtypes", "unchecked"})
+public class DistinctCountCPCAggregationFunction extends BaseSingleInputAggregationFunction<CpcSketch, Comparable> {
Review Comment:
Please add some docstring which documents the usage and behavior.
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountRawCPCAggregationFunction.java:
##########
@@ -0,0 +1,53 @@
+/**
+ * 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.Base64;
+import java.util.List;
+import org.apache.datasketches.cpc.CpcSketch;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+
+
+/**
+ * The {@code DistinctCountRawCPCAggregationFunction} shares the same usage as the
+ * {@link DistinctCountCPCAggregationFunction}, and returns the sketch as a base64 encoded string.
+ */
+public class DistinctCountRawCPCAggregationFunction extends DistinctCountCPCAggregationFunction {
+
+ public DistinctCountRawCPCAggregationFunction(List<ExpressionContext> arguments) {
+ super(arguments);
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.DISTINCTCOUNTRAWCPCSKETCH;
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(CpcSketch sketch) {
+ return Base64.getEncoder().encodeToString(sketch.toByteArray());
+ }
Review Comment:
Would be nice to return a serialized CPC sketch object here which has a custom `compareTo` implementation.
Something like: https://github.com/apache/pinot/blob/master/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/SerializedKLL.java
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1357492853
##########
pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java:
##########
@@ -515,23 +518,23 @@ public VarianceTuple deserialize(ByteBuffer byteBuffer) {
}
};
- public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE
- = new ObjectSerDe<PinotFourthMoment>() {
- @Override
- public byte[] serialize(PinotFourthMoment value) {
- return value.serialize();
- }
+ public static final ObjectSerDe<PinotFourthMoment> PINOT_FOURTH_MOMENT_OBJECT_SER_DE =
+ new ObjectSerDe<PinotFourthMoment>() {
+ @Override
+ public byte[] serialize(PinotFourthMoment value) {
+ return value.serialize();
+ }
- @Override
- public PinotFourthMoment deserialize(byte[] bytes) {
- return PinotFourthMoment.fromBytes(bytes);
- }
+ @Override
+ public PinotFourthMoment deserialize(byte[] bytes) {
+ return PinotFourthMoment.fromBytes(bytes);
+ }
- @Override
- public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
- return PinotFourthMoment.fromBytes(byteBuffer);
- }
- };
+ @Override
+ public PinotFourthMoment deserialize(ByteBuffer byteBuffer) {
+ return PinotFourthMoment.fromBytes(byteBuffer);
+ }
+ };
Review Comment:
I've reverted these additional formatting changes in https://github.com/apache/pinot/pull/11774/commits/0d125dc5682ad0281f1ab1365ec1da731855dcdc
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1355090385
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountRawCPCAggregationFunction.java:
##########
@@ -0,0 +1,53 @@
+/**
+ * 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.Base64;
+import java.util.List;
+import org.apache.datasketches.cpc.CpcSketch;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+
+
+/**
+ * The {@code DistinctCountRawCPCAggregationFunction} shares the same usage as the
+ * {@link DistinctCountCPCAggregationFunction}, and returns the sketch as a base64 encoded string.
+ */
+public class DistinctCountRawCPCAggregationFunction extends DistinctCountCPCAggregationFunction {
+
+ public DistinctCountRawCPCAggregationFunction(List<ExpressionContext> arguments) {
+ super(arguments);
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.DISTINCTCOUNTRAWCPCSKETCH;
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(CpcSketch sketch) {
+ return Base64.getEncoder().encodeToString(sketch.toByteArray());
+ }
Review Comment:
Thanks! I never knew about that convention and will look at including it.
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11774:
URL: https://github.com/apache/pinot/pull/11774
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1355091896
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCAggregationFunction.java:
##########
@@ -0,0 +1,358 @@
+/**
+ * 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.google.common.base.Preconditions;
+import java.util.List;
+import java.util.Map;
+import org.apache.datasketches.cpc.CpcSketch;
+import org.apache.datasketches.cpc.CpcUnion;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.common.ObjectSerDeUtils;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.ObjectGroupByResultHolder;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.utils.CommonConstants;
+
+
+@SuppressWarnings({"rawtypes", "unchecked"})
+public class DistinctCountCPCAggregationFunction extends BaseSingleInputAggregationFunction<CpcSketch, Comparable> {
Review Comment:
Yes, good idea. I would also like to follow up with some changes to the main website documentation if possible.
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on PR #11774:
URL: https://github.com/apache/pinot/pull/11774#issuecomment-1770397374
> Thanks for the contribution! Can you help also add the documentation for this new function?
Yes, I'll try and update the documentation for our earlier contributions 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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on code in PR #11774:
URL: https://github.com/apache/pinot/pull/11774#discussion_r1355129147
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountRawCPCAggregationFunction.java:
##########
@@ -0,0 +1,53 @@
+/**
+ * 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.Base64;
+import java.util.List;
+import org.apache.datasketches.cpc.CpcSketch;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+
+
+/**
+ * The {@code DistinctCountRawCPCAggregationFunction} shares the same usage as the
+ * {@link DistinctCountCPCAggregationFunction}, and returns the sketch as a base64 encoded string.
+ */
+public class DistinctCountRawCPCAggregationFunction extends DistinctCountCPCAggregationFunction {
+
+ public DistinctCountRawCPCAggregationFunction(List<ExpressionContext> arguments) {
+ super(arguments);
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.DISTINCTCOUNTRAWCPCSKETCH;
+ }
+
+ @Override
+ public ColumnDataType getFinalResultColumnType() {
+ return ColumnDataType.STRING;
+ }
+
+ @Override
+ public String extractFinalResult(CpcSketch sketch) {
+ return Base64.getEncoder().encodeToString(sketch.toByteArray());
+ }
Review Comment:
I included a very basic version here: https://github.com/apache/pinot/pull/11774/commits/cb2dfa47bf7e2b1b11a557e86b698fbd3862a324
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on PR #11774:
URL: https://github.com/apache/pinot/pull/11774#issuecomment-1786847023
> Thanks for the contribution! Can you help also add the documentation for this new function?
Hi @Jackie-Jiang I have prepared some documentation here for review:
https://github.com/pinot-contrib/pinot-docs/pull/254
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11774:
URL: https://github.com/apache/pinot/pull/11774#issuecomment-1755683900
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11774](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (05ed526) into [master](https://app.codecov.io/gh/apache/pinot/commit/24af80d3bccdb4c2b86cae392ea022b824db1c1a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (24af80d) will **decrease** coverage by `0.08%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11774 +/- ##
============================================
- Coverage 14.45% 14.38% -0.08%
Complexity 201 201
============================================
Files 2342 2346 +4
Lines 125917 126225 +308
Branches 19370 19442 +72
============================================
- Hits 18205 18155 -50
- Misses 106170 106536 +366
+ Partials 1542 1534 -8
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.37% <0.00%> (-0.05%)` | :arrow_down: |
| [java-17](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.37% <0.00%> (-0.07%)` | :arrow_down: |
| [java-20](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.38% <0.00%> (-0.04%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.38% <0.00%> (-0.08%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.38% <0.00%> (-0.08%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11774/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.38% <0.00%> (-0.08%)` | :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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
| [.../processing/aggregator/ValueAggregatorFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvYWdncmVnYXRvci9WYWx1ZUFnZ3JlZ2F0b3JGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...gregation/function/AggregationFunctionFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...gator/DistinctCountThetaSketchValueAggregator.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL0Rpc3RpbmN0Q291bnRUaGV0YVNrZXRjaFZhbHVlQWdncmVnYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...gment/local/aggregator/ValueAggregatorFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9hZ2dyZWdhdG9yL1ZhbHVlQWdncmVnYXRvckZhY3RvcnkuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...aggregator/DistinctCountThetaSketchAggregator.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvYWdncmVnYXRvci9EaXN0aW5jdENvdW50VGhldGFTa2V0Y2hBZ2dyZWdhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...nction/DistinctCountRawCPCAggregationFunction.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50UmF3Q1BDQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...he/pinot/segment/local/utils/CustomSerDeUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9DdXN0b21TZXJEZVV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...cessing/aggregator/DistinctCountCPCAggregator.java](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3Byb2Nlc3NpbmcvYWdncmVnYXRvci9EaXN0aW5jdENvdW50Q1BDQWdncmVnYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [5 more](https://app.codecov.io/gh/apache/pinot/pull/11774?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [7 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11774/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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
Re: [PR] Issue-6420 Add support for Apache Datasketches CPC sketch [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #11774:
URL: https://github.com/apache/pinot/pull/11774#issuecomment-1769284059
Thanks for the contribution! Can you help also add the documentation for this new function?
--
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