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 2020/09/23 12:45:12 UTC
[GitHub] [incubator-pinot] KKcorps opened a new pull request #6053: Add support for big decimal data type
KKcorps opened a new pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053
The PR addresses the issue #5904.
It adds the support for UDFs to convert hex presentation to big decimal and vice-versa and also allows user to perform addition with fixed precision.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496870036
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
Review comment:
This is not the hex string. This converts to normal String e.g. BigDecimal(1234121.5123127182, 2) to "1234.512"
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494816355
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
* Yes, it is the 2's complement representation of underling unscaled value
* Scale is the first 4 bytes in the byte array, the remaining bytes are of unscaled value
* The unscaled value byte array is indeed Big-endian and they are copied as such in the result array. There's no need to swap the bytes.
In summary, the final byte array is just unscaled value's bytes appended to scale's bytes
----------------------------------------------------------------
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.
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] [incubator-pinot] kishoreg commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497053467
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
Review comment:
Got it. please add java docs
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision) {
+ super(expression);
+ _mathContext = new MathContext(precision);
+ }
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision, Integer scale) {
+ super(expression);
+ _mathContext = new MathContext(precision);
+ _scale = scale;
+ }
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression) {
+ super(expression);
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ sumValue = sumValue.add(value, _mathContext);
+ }
+ aggregationResultHolder.setValue(setScale(sumValue));
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ for (int i = 0; i < length; i++) {
+ int groupKey = groupKeyArray[i];
+ BigDecimal groupByResultValue = getDefaultResult(groupByResultHolder, groupKey);
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ groupByResultValue = groupByResultValue.add(value, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, setScale(groupByResultValue));
+ }
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ for (int i = 0; i < length; i++) {
+ byte[] value = valueArray[i];
+ for (int groupKey : groupKeysArray[i]) {
+ BigDecimal groupByResultValue = getDefaultResult(groupByResultHolder, groupKey);
+ BigDecimal valueBigDecimal = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(value));
+ groupByResultValue = groupByResultValue.add(valueBigDecimal, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, setScale(groupByResultValue));
+ }
+ }
+ }
+
+ @Override
+ public BigDecimal extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return getDefaultResult(aggregationResultHolder);
+ }
+
+ @Override
+ public BigDecimal extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return getDefaultResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public BigDecimal merge(BigDecimal intermediateResult1, BigDecimal intermediateResult2) {
+ try {
+ return setScale(intermediateResult1.add(intermediateResult2, _mathContext));
+ } catch (Exception e) {
+ throw new RuntimeException("Caught Exception while merging results in sum with precision function", e);
+ }
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return true;
+ }
+
+ @Override
+ public DataSchema.ColumnDataType getIntermediateResultColumnType() {
+ return DataSchema.ColumnDataType.OBJECT;
+ }
+
+ @Override
+ public DataSchema.ColumnDataType getFinalResultColumnType() {
+ return DataSchema.ColumnDataType.STRING;
+ }
+
+ @Override
+ public BigDecimal extractFinalResult(BigDecimal intermediateResult) {
+ return intermediateResult;
+ }
+
+ public BigDecimal getDefaultResult(AggregationResultHolder aggregationResultHolder) {
+ BigDecimal result = aggregationResultHolder.getResult();
+ if (result == null) {
+ result = new BigDecimal(0, _mathContext);
+ aggregationResultHolder.setValue(result);
+ }
+ result = setScale(result);
+ return result;
+ }
+
+ public BigDecimal getDefaultResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ BigDecimal result = groupByResultHolder.getResult(groupKey);
+ if (result == null) {
+ result = new BigDecimal(0, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, result);
+ }
+ result = setScale(result);
+ return result;
+ }
+
+ private BigDecimal setScale(BigDecimal value) {
Review comment:
got it. I am assuming big decimal additions are idempotent -- bd1 + bd2 same as bd2 + bd1
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r495422139
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
@KKcorps , can we add some e2e query execution functional tests?
----------------------------------------------------------------
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.
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] [incubator-pinot] codecov-commenter commented on pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-700933500
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=h1) Report
> Merging [#6053](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) will **increase** coverage by `6.29%`.
> The diff coverage is `59.40%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6053 +/- ##
==========================================
+ Coverage 66.44% 72.74% +6.29%
==========================================
Files 1075 1214 +139
Lines 54773 56725 +1952
Branches 8168 8245 +77
==========================================
+ Hits 36396 41264 +4868
+ Misses 15700 12783 -2917
- Partials 2677 2678 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| #integration | `46.02% <48.11%> (?)` | |
| #unittests | `63.77% <37.82%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
| [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `53.33% <0.00%> (-3.81%)` | :arrow_down: |
| [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: |
| [.../org/apache/pinot/client/ResultTableResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFRhYmxlUmVzdWx0U2V0LmphdmE=) | `24.00% <0.00%> (-10.29%)` | :arrow_down: |
| [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: |
| [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: |
| [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `96.61% <ø> (-3.39%)` | :arrow_down: |
| [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | |
| [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: |
| ... and [942 more](https://codecov.io/gh/apache/incubator-pinot/pull/6053/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=footer). Last update [2379791...d3add2c](https://codecov.io/gh/apache/incubator-pinot/pull/6053?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r493727370
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
Review comment:
no, that is in the scalar 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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496982316
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java
##########
@@ -776,31 +781,30 @@ public IdSet deserialize(ByteBuffer byteBuffer) {
}
};
+ public static final ObjectSerDe<BigDecimal> BIGDECIMAL_SER_DE = new ObjectSerDe<BigDecimal>() {
+
+ @Override
+ public byte[] serialize(BigDecimal value) {
+ return DataTypeConversionFunctions.bigDecimalToBytes(value);
+ }
+
+ @Override
+ public BigDecimal deserialize(byte[] bytes) {
+ return new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(bytes));
+ }
+
+ @Override
+ public BigDecimal deserialize(ByteBuffer byteBuffer) {
+ byte[] bytes = new byte[byteBuffer.remaining()];
+ byteBuffer.get(bytes);
+ return deserialize(bytes);
+ }
+ };
+
// NOTE: DO NOT change the order, it has to be the same order as the ObjectType
//@formatter:off
- private static final ObjectSerDe[] SER_DES = {
- STRING_SER_DE,
- LONG_SER_DE,
- DOUBLE_SER_DE,
- DOUBLE_ARRAY_LIST_SER_DE,
- AVG_PAIR_SER_DE,
- MIN_MAX_RANGE_PAIR_SER_DE,
- HYPER_LOG_LOG_SER_DE,
- QUANTILE_DIGEST_SER_DE,
- MAP_SER_DE,
- INT_SET_SER_DE,
- TDIGEST_SER_DE,
- DISTINCT_TABLE_SER_DE,
- DATA_SKETCH_SER_DE,
- GEOMETRY_SER_DE,
- ROARING_BITMAP_SER_DE,
- LONG_SET_SER_DE,
- FLOAT_SET_SER_DE,
- DOUBLE_SET_SER_DE,
- STRING_SET_SER_DE,
- BYTES_SET_SER_DE,
- ID_SET_SER_DE
- };
+ private static final ObjectSerDe[] SER_DES =
Review comment:
resolved.
----------------------------------------------------------------
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.
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] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497712826
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
Review comment:
I feel string type is easier to use compared to bytes, where the raw data is the numeric strings
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497565255
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
Review comment:
That can be done later when we implement full-fledged Big decimal support in DB itself.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497550423
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ sumValue = sumValue.add(value, _mathContext);
+ }
+ aggregationResultHolder.setValue(setScale(sumValue));
Review comment:
resolved.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494462257
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -79,7 +79,10 @@
// Geo relationship
ST_CONTAINS("ST_Contains"),
- ST_EQUALS("ST_Equals");
+ ST_EQUALS("ST_Equals"),
+
+ //Big decimal
+ ADD_WITH_PRECISION("add_with_precision");
Review comment:
Fixed.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496623778
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/AggregationFunctionType.java
##########
@@ -30,6 +30,7 @@
MIN("min"),
MAX("max"),
SUM("sum"),
+ SUMPRECISION("sumPrecision"),
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496976524
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java
##########
@@ -117,6 +117,18 @@ public static AggregationFunction getAggregationFunction(FunctionContext functio
return new MaxAggregationFunction(firstArgument);
case SUM:
return new SumAggregationFunction(firstArgument);
+ case SUMPRECISION:
+ int numArguments = arguments.size();
+ if (numArguments == 3) {
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.
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] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497711189
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
Review comment:
`DataTypeConversionFunctions.bytesToBigDecimal()` first creates the `BigDecimal`, then create a `String` from it. Here we convert `String` back to `BigDecimal` again. Instead, we can have a util method to get `BigDecimal` from `byte[]` to avoid the unnecessary `BigDecimal` -> `String` -> `BigDecimal`.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497564520
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
Review comment:
I don't understand. What problem would Util class solve?
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496962171
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
Review comment:
This doesn't output the hex string. It outputs BigDecimal.toString.
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698732361
The PR title is a bit misleading. This PR doesn't add support for DECIMAL with some precision and scale as a first class data type in Pinot like we have other primitives. We are adding a transform 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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494796477
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/AggregationFunctionType.java
##########
@@ -30,6 +30,7 @@
MIN("min"),
MAX("max"),
SUM("sum"),
+ SUMPRECISION("sumPrecision"),
Review comment:
Yes, I was actually discussing this with @kishoreg
I can allow both.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494462493
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
+
+ private List<TransformFunction> _transformFunctions = new ArrayList<>();
+ private BigDecimal[] _sums;
+ private Integer _precision = null;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ // Check that there are more than 1 arguments
+ if (arguments.size() < 3) {
+ throw new IllegalArgumentException("At least 3 arguments are required for ADD transform function");
+ }
+
+ for (TransformFunction argument : arguments) {
+ if (argument instanceof LiteralTransformFunction) {
+ if (_precision != null) {
+ throw new IllegalArgumentException("Only one precision value can be specified in ADD transform function");
+ }
+ _precision = Integer.parseInt(((LiteralTransformFunction) argument).getLiteral());
+ } else {
+ if (!argument.getResultMetadata().isSingleValue()) {
+ throw new IllegalArgumentException("All the arguments of ADD transform function must be single-valued");
+ }
+ _transformFunctions.add(argument);
+ }
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BYTES_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public byte[][] transformToBytesValuesSV(ProjectionBlock projectionBlock) {
+ if (_sums == null) {
+ _sums = new BigDecimal[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+
+ int length = projectionBlock.getNumDocs();
+ Arrays.fill(_sums, 0, length, new BigDecimal(0));
+ MathContext mathContext;
+ if (_precision != null) {
Review comment:
resolved.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496868587
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision) {
+ super(expression);
+ _mathContext = new MathContext(precision);
+ }
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision, Integer scale) {
+ super(expression);
+ _mathContext = new MathContext(precision);
+ _scale = scale;
+ }
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression) {
+ super(expression);
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ sumValue = sumValue.add(value, _mathContext);
+ }
+ aggregationResultHolder.setValue(setScale(sumValue));
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ for (int i = 0; i < length; i++) {
+ int groupKey = groupKeyArray[i];
+ BigDecimal groupByResultValue = getDefaultResult(groupByResultHolder, groupKey);
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ groupByResultValue = groupByResultValue.add(value, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, setScale(groupByResultValue));
+ }
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ for (int i = 0; i < length; i++) {
+ byte[] value = valueArray[i];
+ for (int groupKey : groupKeysArray[i]) {
+ BigDecimal groupByResultValue = getDefaultResult(groupByResultHolder, groupKey);
+ BigDecimal valueBigDecimal = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(value));
+ groupByResultValue = groupByResultValue.add(valueBigDecimal, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, setScale(groupByResultValue));
+ }
+ }
+ }
+
+ @Override
+ public BigDecimal extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return getDefaultResult(aggregationResultHolder);
+ }
+
+ @Override
+ public BigDecimal extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return getDefaultResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public BigDecimal merge(BigDecimal intermediateResult1, BigDecimal intermediateResult2) {
+ try {
+ return setScale(intermediateResult1.add(intermediateResult2, _mathContext));
+ } catch (Exception e) {
+ throw new RuntimeException("Caught Exception while merging results in sum with precision function", e);
+ }
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return true;
+ }
+
+ @Override
+ public DataSchema.ColumnDataType getIntermediateResultColumnType() {
+ return DataSchema.ColumnDataType.OBJECT;
+ }
+
+ @Override
+ public DataSchema.ColumnDataType getFinalResultColumnType() {
+ return DataSchema.ColumnDataType.STRING;
+ }
+
+ @Override
+ public BigDecimal extractFinalResult(BigDecimal intermediateResult) {
+ return intermediateResult;
+ }
+
+ public BigDecimal getDefaultResult(AggregationResultHolder aggregationResultHolder) {
+ BigDecimal result = aggregationResultHolder.getResult();
+ if (result == null) {
+ result = new BigDecimal(0, _mathContext);
+ aggregationResultHolder.setValue(result);
+ }
+ result = setScale(result);
+ return result;
+ }
+
+ public BigDecimal getDefaultResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ BigDecimal result = groupByResultHolder.getResult(groupKey);
+ if (result == null) {
+ result = new BigDecimal(0, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, result);
+ }
+ result = setScale(result);
+ return result;
+ }
+
+ private BigDecimal setScale(BigDecimal value) {
Review comment:
If the user doesn't set scale, then just return the value as such otherwise set the scale value.
Just a small helper method to avoid if else in the 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.
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] [incubator-pinot] kishoreg merged pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
kishoreg merged pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r495465629
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
@siddharthteotia Yes, the tests are almost done. Will push them tomorrow.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497551575
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
Review comment:
Won't the loop be unrolled anyways? Reduced the bytes to 2
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496961334
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
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.
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] [incubator-pinot] siddharthteotia edited a comment on pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia edited a comment on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698732361
The PR title is a bit misleading. This PR doesn't add support for DECIMAL with some precision and scale as a first class data type in Pinot like we have other primitives. Storing DECIMAL column type physically is not yet supported. I suggest changing the title accordingly as it will go into release notes.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698785869
@siddharthteotia Changed the title
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496982041
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
+
+ private List<TransformFunction> _transformFunctions = new ArrayList<>();
+ private BigDecimal[] _sums;
+ private Integer _precision = null;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ // Check that there are more than 1 arguments
+ if (arguments.size() < 3) {
+ throw new IllegalArgumentException("At least 3 arguments are required for ADD transform function");
+ }
+
+ for (TransformFunction argument : arguments) {
+ if (argument instanceof LiteralTransformFunction) {
Review comment:
Not applicable now.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497550985
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
+ StringBuilder sb = new StringBuilder();
+ for (byte b : bytes) {
+ sb.append(String.format("%02X ", b));
+ }
+
+ return sb.toString();
+ }
+
+ @ScalarFunction
+ public static String base64Encode(String input) {
+ return Base64.getEncoder().encodeToString(input.getBytes());
+ }
+
+ @ScalarFunction
+ public static String base64Decode(String input) {
+ return new String(Base64.getDecoder().decode(input.getBytes()));
+ }
Review comment:
done
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
+ StringBuilder sb = new StringBuilder();
+ for (byte b : bytes) {
+ sb.append(String.format("%02X ", b));
+ }
+
+ return sb.toString();
+ }
+
+ @ScalarFunction
+ public static String base64Encode(String input) {
+ return Base64.getEncoder().encodeToString(input.getBytes());
+ }
Review comment:
done
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
Review comment:
done
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494462257
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -79,7 +79,10 @@
// Geo relationship
ST_CONTAINS("ST_Contains"),
- ST_EQUALS("ST_Equals");
+ ST_EQUALS("ST_Equals"),
+
+ //Big decimal
+ ADD_WITH_PRECISION("add_with_precision");
Review comment:
Fixed.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
+
+ private List<TransformFunction> _transformFunctions = new ArrayList<>();
+ private BigDecimal[] _sums;
+ private Integer _precision = null;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ // Check that there are more than 1 arguments
+ if (arguments.size() < 3) {
+ throw new IllegalArgumentException("At least 3 arguments are required for ADD transform function");
+ }
+
+ for (TransformFunction argument : arguments) {
+ if (argument instanceof LiteralTransformFunction) {
+ if (_precision != null) {
+ throw new IllegalArgumentException("Only one precision value can be specified in ADD transform function");
+ }
+ _precision = Integer.parseInt(((LiteralTransformFunction) argument).getLiteral());
+ } else {
+ if (!argument.getResultMetadata().isSingleValue()) {
+ throw new IllegalArgumentException("All the arguments of ADD transform function must be single-valued");
+ }
+ _transformFunctions.add(argument);
+ }
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BYTES_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public byte[][] transformToBytesValuesSV(ProjectionBlock projectionBlock) {
+ if (_sums == null) {
+ _sums = new BigDecimal[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+
+ int length = projectionBlock.getNumDocs();
+ Arrays.fill(_sums, 0, length, new BigDecimal(0));
+ MathContext mathContext;
+ if (_precision != null) {
Review comment:
resolved.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/AggregationFunctionType.java
##########
@@ -30,6 +30,7 @@
MIN("min"),
MAX("max"),
SUM("sum"),
+ SUMPRECISION("sumPrecision"),
Review comment:
Yes, I was actually discussing this with @kishoreg
I can allow both.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
* Yes, it is the 2's complement representation of underling unscaled value
* Scale is the first 4 bytes in the byte array, the remaining bytes are of unscaled value
* The unscaled value byte array is indeed Big-endian and they are copied as such in the result array. There's no need to swap the bytes.
In summary, the final byte array is just unscaled value's bytes appended to scale's bytes
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496867698
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision) {
Review comment:
Yes, they can differ.
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494758942
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
Review comment:
This should be in BIG ENDIAN byte order
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
- What is the format here? Are we saying that byte array is the 2's complement representation of the underlying unscaled value.
- How is scale represented in the byte array?
- Do we have to take care of endianness here? The unscaled values is going to be BIG-ENDIAN. How's it working without swapping bytes?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/AggregationFunctionType.java
##########
@@ -30,6 +30,7 @@
MIN("min"),
MAX("max"),
SUM("sum"),
+ SUMPRECISION("sumPrecision"),
Review comment:
Why not allow both precision and scale?
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698785869
@siddharthteotia Changed the title
----------------------------------------------------------------
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.
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] [incubator-pinot] kishoreg commented on pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-702349449
Looks good merging it. Lets learn from this v0 version and incorporate the learnings when we add BigDecimal as a data type
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494759048
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
- What is the format here? Are we saying that byte array is the 2's complement representation of the underlying unscaled value.
- How is scale represented in the byte array?
- Do we have to take care of endianness here? The unscaled values is going to be BIG-ENDIAN. How's it working without swapping bytes?
----------------------------------------------------------------
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.
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] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r493820414
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
+
+ private List<TransformFunction> _transformFunctions = new ArrayList<>();
+ private BigDecimal[] _sums;
+ private Integer _precision = null;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ // Check that there are more than 1 arguments
+ if (arguments.size() < 3) {
+ throw new IllegalArgumentException("At least 3 arguments are required for ADD transform function");
+ }
+
+ for (TransformFunction argument : arguments) {
+ if (argument instanceof LiteralTransformFunction) {
Review comment:
Parse the last argument as the precision instead of identifying `LITERAL` as precision? In certain cases user can add `LITERAL` to a column, e.g. `addWithPrecision(col, BigDecimal(123), 0)`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
+
+ private List<TransformFunction> _transformFunctions = new ArrayList<>();
+ private BigDecimal[] _sums;
+ private Integer _precision = null;
+
+ @Override
+ public String getName() {
+ return FUNCTION_NAME;
+ }
+
+ @Override
+ public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
+ // Check that there are more than 1 arguments
+ if (arguments.size() < 3) {
+ throw new IllegalArgumentException("At least 3 arguments are required for ADD transform function");
+ }
+
+ for (TransformFunction argument : arguments) {
+ if (argument instanceof LiteralTransformFunction) {
+ if (_precision != null) {
+ throw new IllegalArgumentException("Only one precision value can be specified in ADD transform function");
+ }
+ _precision = Integer.parseInt(((LiteralTransformFunction) argument).getLiteral());
+ } else {
+ if (!argument.getResultMetadata().isSingleValue()) {
+ throw new IllegalArgumentException("All the arguments of ADD transform function must be single-valued");
+ }
+ _transformFunctions.add(argument);
+ }
+ }
+ }
+
+ @Override
+ public TransformResultMetadata getResultMetadata() {
+ return BYTES_SV_NO_DICTIONARY_METADATA;
+ }
+
+ @Override
+ public byte[][] transformToBytesValuesSV(ProjectionBlock projectionBlock) {
+ if (_sums == null) {
+ _sums = new BigDecimal[DocIdSetPlanNode.MAX_DOC_PER_CALL];
+ }
+
+ int length = projectionBlock.getNumDocs();
+ Arrays.fill(_sums, 0, length, new BigDecimal(0));
+ MathContext mathContext;
+ if (_precision != null) {
Review comment:
Function takes at least 3 arguments, and precision is mandatory?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -79,7 +79,10 @@
// Geo relationship
ST_CONTAINS("ST_Contains"),
- ST_EQUALS("ST_Equals");
+ ST_EQUALS("ST_Equals"),
+
+ //Big decimal
+ ADD_WITH_PRECISION("add_with_precision");
Review comment:
@kishoreg This is the transform, instead of the aggregation. We might want to use `add_precision` for transform and `sum_precision` for 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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497561187
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r498436463
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
Review comment:
ok. added the methods in the existing class itself.
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494759161
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/AggregationFunctionType.java
##########
@@ -30,6 +30,7 @@
MIN("min"),
MAX("max"),
SUM("sum"),
+ SUMPRECISION("sumPrecision"),
Review comment:
Why not allow both precision and scale?
----------------------------------------------------------------
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.
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] [incubator-pinot] kishoreg commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497694103
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
Review comment:
let's not try to optimize this for now. we can consider these optimizations when we add first class support for bigdecimal type
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494758942
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
Review comment:
This should be in BIG ENDIAN byte order
----------------------------------------------------------------
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.
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] [incubator-pinot] kishoreg commented on a change in pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r493701223
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
##########
@@ -79,7 +79,10 @@
// Geo relationship
ST_CONTAINS("ST_Contains"),
- ST_EQUALS("ST_Equals");
+ ST_EQUALS("ST_Equals"),
+
+ //Big decimal
+ ADD_WITH_PRECISION("add_with_precision");
Review comment:
call this sum_precision
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
Review comment:
add java docs with sample call
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AdditionWithPrecisionTransformFunction.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.operator.transform.function;
+
+import java.math.BigDecimal;
+import java.math.MathContext;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.operator.blocks.ProjectionBlock;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+
+
+public class AdditionWithPrecisionTransformFunction extends BaseTransformFunction {
+
+ public static final String FUNCTION_NAME = "addWithPrecision";
Review comment:
this is toBigDecimal transform function right and can be used for sum, min max 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.
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] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r497201010
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
Review comment:
Similarly here
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
Review comment:
For better performance, avoid the for loop
```suggestion
bigDecimalBytesArray[0] = (byte) scale >>> 24;
bigDecimalBytesArray[1] = (byte) scale >>> 16;
bigDecimalBytesArray[2] = (byte) scale >>> 8;
bigDecimalBytesArray[3] = (byte) scale;
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
Review comment:
You may use `BytesUtils.toBytes(hex)`
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
+ StringBuilder sb = new StringBuilder();
Review comment:
`BytesUtils.toHexString(bytes)`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
Review comment:
We might also want to support string type as the BigDecimal values?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ sumValue = sumValue.add(value, _mathContext);
+ }
+ aggregationResultHolder.setValue(setScale(sumValue));
Review comment:
To get the consistent result, we should either set the scale at the end (in `extractFinalResult`) or for each value. I think setting it in `extractFinalResult` should give better performance.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
+ StringBuilder sb = new StringBuilder();
+ for (byte b : bytes) {
+ sb.append(String.format("%02X ", b));
+ }
+
+ return sb.toString();
+ }
+
+ @ScalarFunction
+ public static String base64Encode(String input) {
+ return Base64.getEncoder().encodeToString(input.getBytes());
+ }
+
+ @ScalarFunction
+ public static String base64Decode(String input) {
+ return new String(Base64.getDecoder().decode(input.getBytes()));
+ }
Review comment:
```suggestion
public static byte[] base64Decode(String input) {
return Base64.getDecoder().decode(input);
}
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
Review comment:
Recommend passing in `String` instead of `BigDecimal` for symmetry with `bytesToBigDecimal()`. Also, scaler transform function won't work on `BigDecimal` objects.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
+ StringBuilder sb = new StringBuilder();
+ for (byte b : bytes) {
+ sb.append(String.format("%02X ", b));
+ }
+
+ return sb.toString();
+ }
+
+ @ScalarFunction
+ public static String base64Encode(String input) {
+ return Base64.getEncoder().encodeToString(input.getBytes());
+ }
Review comment:
```suggestion
public static byte[] base64Encode(byte[] bytes) {
return Base64.getEncoder().encodeToString(bytes);
}
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
+ int numArguments = arguments.size();
+
+ if (numArguments == 3) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _scale = Integer.parseInt(arguments.get(2).getLiteral());
+ _mathContext = new MathContext(precision);
+ } else if (numArguments == 2) {
+ Integer precision = Integer.parseInt(arguments.get(1).getLiteral());
+ _mathContext = new MathContext(precision);
+ }
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
Review comment:
Suggest adding a util class for `BigDecimal` (the scalar function can also call the util class). Here we should avoid converting `BigDecimal` to `String` and back again
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
Review comment:
Please add some javadoc on the arguments expected for the function.
Also recommend renaming it to `SumPrecisionAggregationFunction` to match the function name
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
Review comment:
Make these 2 variables `private final` because the aggregation function can be shared among multiple segments, and needs to be stateless.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,92 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Base64;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
Review comment:
Actually, after another thought, I don't think we need to use 4 bytes for the scale. Scale larger than 256 doesn't make lots of sense.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, List<ExpressionContext> arguments) {
+ super(expression);
Review comment:
```suggestion
public SumWithPrecisionAggregationFunction(List<ExpressionContext> arguments) {
super(arguments.get(0));
```
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia edited a comment on pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia edited a comment on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698732361
The PR title is a bit misleading. This PR doesn't add support for DECIMAL with some precision and scale as a first class data type in Pinot like we have other primitives. Storing DECIMAL column type physically is not yet supported. I suggest changing the title accordingly as it will go into release notes.
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r494816355
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
* Yes, it is the 2's complement representation of underlying unscaled value
* Scale is the first 4 bytes in the byte array, the remaining bytes are of unscaled value
* The unscaled value byte array is indeed Big-endian and they are copied as such in the result array. There's no need to swap the bytes.
In summary, the final byte array is just unscaled value's bytes appended to scale's bytes
----------------------------------------------------------------
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.
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] [incubator-pinot] siddharthteotia commented on pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698732361
The PR title is a bit misleading. This PR doesn't add support for DECIMAL with some precision and scale as a first class data type in Pinot like we have other primitives. We are adding a transform 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.
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] [incubator-pinot] siddharthteotia commented on pull request #6053: Add support for big decimal data type
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#issuecomment-698076320
I would also like to review this PR. Please give me time until tomorrow.
----------------------------------------------------------------
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.
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] [incubator-pinot] kishoreg commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496850851
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
Review comment:
rename to bytesToBigDecimalHex?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
Review comment:
whats is happening inside number.toString? is it possible for us to generate the hex string without having to create big integer and big decimal?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java
##########
@@ -117,6 +117,18 @@ public static AggregationFunction getAggregationFunction(FunctionContext functio
return new MaxAggregationFunction(firstArgument);
case SUM:
return new SumAggregationFunction(firstArgument);
+ case SUMPRECISION:
+ int numArguments = arguments.size();
+ if (numArguments == 3) {
Review comment:
its probably better to move this logic into SumPrecission(arguments) constructor. You can still keep the other two constructors
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,81 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
+ System.arraycopy(bytes, 4, vals, 0, vals.length);
+ BigInteger unscaled = new BigInteger(vals);
+ BigDecimal number = new BigDecimal(unscaled, scale);
+ return number.toString();
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalFromString(String bigDecimal) {
+ return bigDecimalToBytes(new BigDecimal(bigDecimal));
+ }
+
+ @ScalarFunction
+ public static byte[] hexToBytes(String hex) {
+ int len = hex.length();
+ byte[] data = new byte[len / 2];
+ for (int i = 0; i < len; i += 2) {
+ data[i / 2] = (byte) ((Character.digit(hex.charAt(i), 16) << 4) + Character.digit(hex.charAt(i + 1), 16));
+ }
+ return data;
+ }
+
+ @ScalarFunction
+ public static String bytesToHex(byte[] bytes) {
Review comment:
while you are there can you also add base64encode decode?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/common/ObjectSerDeUtils.java
##########
@@ -776,31 +781,30 @@ public IdSet deserialize(ByteBuffer byteBuffer) {
}
};
+ public static final ObjectSerDe<BigDecimal> BIGDECIMAL_SER_DE = new ObjectSerDe<BigDecimal>() {
+
+ @Override
+ public byte[] serialize(BigDecimal value) {
+ return DataTypeConversionFunctions.bigDecimalToBytes(value);
+ }
+
+ @Override
+ public BigDecimal deserialize(byte[] bytes) {
+ return new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(bytes));
+ }
+
+ @Override
+ public BigDecimal deserialize(ByteBuffer byteBuffer) {
+ byte[] bytes = new byte[byteBuffer.remaining()];
+ byteBuffer.get(bytes);
+ return deserialize(bytes);
+ }
+ };
+
// NOTE: DO NOT change the order, it has to be the same order as the ObjectType
//@formatter:off
- private static final ObjectSerDe[] SER_DES = {
- STRING_SER_DE,
- LONG_SER_DE,
- DOUBLE_SER_DE,
- DOUBLE_ARRAY_LIST_SER_DE,
- AVG_PAIR_SER_DE,
- MIN_MAX_RANGE_PAIR_SER_DE,
- HYPER_LOG_LOG_SER_DE,
- QUANTILE_DIGEST_SER_DE,
- MAP_SER_DE,
- INT_SET_SER_DE,
- TDIGEST_SER_DE,
- DISTINCT_TABLE_SER_DE,
- DATA_SKETCH_SER_DE,
- GEOMETRY_SER_DE,
- ROARING_BITMAP_SER_DE,
- LONG_SET_SER_DE,
- FLOAT_SET_SER_DE,
- DOUBLE_SET_SER_DE,
- STRING_SET_SER_DE,
- BYTES_SET_SER_DE,
- ID_SET_SER_DE
- };
+ private static final ObjectSerDe[] SER_DES =
Review comment:
check your IDE pref to honor @formatter:off annotation
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision) {
+ super(expression);
+ _mathContext = new MathContext(precision);
+ }
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision, Integer scale) {
+ super(expression);
+ _mathContext = new MathContext(precision);
+ _scale = scale;
+ }
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression) {
+ super(expression);
+ }
+
+ @Override
+ public AggregationFunctionType getType() {
+ return AggregationFunctionType.SUMPRECISION;
+ }
+
+ @Override
+ public AggregationResultHolder createAggregationResultHolder() {
+ return new ObjectAggregationResultHolder();
+ }
+
+ @Override
+ public GroupByResultHolder createGroupByResultHolder(int initialCapacity, int maxCapacity) {
+ return new ObjectGroupByResultHolder(initialCapacity, maxCapacity);
+ }
+
+ @Override
+ public void aggregate(int length, AggregationResultHolder aggregationResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ BigDecimal sumValue = getDefaultResult(aggregationResultHolder);
+ for (int i = 0; i < length; i++) {
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ sumValue = sumValue.add(value, _mathContext);
+ }
+ aggregationResultHolder.setValue(setScale(sumValue));
+ }
+
+ @Override
+ public void aggregateGroupBySV(int length, int[] groupKeyArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ for (int i = 0; i < length; i++) {
+ int groupKey = groupKeyArray[i];
+ BigDecimal groupByResultValue = getDefaultResult(groupByResultHolder, groupKey);
+ BigDecimal value = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(valueArray[i]));
+ groupByResultValue = groupByResultValue.add(value, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, setScale(groupByResultValue));
+ }
+ }
+
+ @Override
+ public void aggregateGroupByMV(int length, int[][] groupKeysArray, GroupByResultHolder groupByResultHolder,
+ Map<ExpressionContext, BlockValSet> blockValSetMap) {
+ byte[][] valueArray = blockValSetMap.get(_expression).getBytesValuesSV();
+ for (int i = 0; i < length; i++) {
+ byte[] value = valueArray[i];
+ for (int groupKey : groupKeysArray[i]) {
+ BigDecimal groupByResultValue = getDefaultResult(groupByResultHolder, groupKey);
+ BigDecimal valueBigDecimal = new BigDecimal(DataTypeConversionFunctions.bytesToBigDecimal(value));
+ groupByResultValue = groupByResultValue.add(valueBigDecimal, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, setScale(groupByResultValue));
+ }
+ }
+ }
+
+ @Override
+ public BigDecimal extractAggregationResult(AggregationResultHolder aggregationResultHolder) {
+ return getDefaultResult(aggregationResultHolder);
+ }
+
+ @Override
+ public BigDecimal extractGroupByResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ return getDefaultResult(groupByResultHolder, groupKey);
+ }
+
+ @Override
+ public BigDecimal merge(BigDecimal intermediateResult1, BigDecimal intermediateResult2) {
+ try {
+ return setScale(intermediateResult1.add(intermediateResult2, _mathContext));
+ } catch (Exception e) {
+ throw new RuntimeException("Caught Exception while merging results in sum with precision function", e);
+ }
+ }
+
+ @Override
+ public boolean isIntermediateResultComparable() {
+ return true;
+ }
+
+ @Override
+ public DataSchema.ColumnDataType getIntermediateResultColumnType() {
+ return DataSchema.ColumnDataType.OBJECT;
+ }
+
+ @Override
+ public DataSchema.ColumnDataType getFinalResultColumnType() {
+ return DataSchema.ColumnDataType.STRING;
+ }
+
+ @Override
+ public BigDecimal extractFinalResult(BigDecimal intermediateResult) {
+ return intermediateResult;
+ }
+
+ public BigDecimal getDefaultResult(AggregationResultHolder aggregationResultHolder) {
+ BigDecimal result = aggregationResultHolder.getResult();
+ if (result == null) {
+ result = new BigDecimal(0, _mathContext);
+ aggregationResultHolder.setValue(result);
+ }
+ result = setScale(result);
+ return result;
+ }
+
+ public BigDecimal getDefaultResult(GroupByResultHolder groupByResultHolder, int groupKey) {
+ BigDecimal result = groupByResultHolder.getResult(groupKey);
+ if (result == null) {
+ result = new BigDecimal(0, _mathContext);
+ groupByResultHolder.setValueForKey(groupKey, result);
+ }
+ result = setScale(result);
+ return result;
+ }
+
+ private BigDecimal setScale(BigDecimal value) {
Review comment:
this method is bit confusing, what is it trying to do. add some javadocs and maybe rename the function if possible
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumWithPrecisionAggregationFunction.java
##########
@@ -0,0 +1,174 @@
+/**
+ * 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.math.BigDecimal;
+import java.math.MathContext;
+import java.util.Map;
+import org.apache.pinot.common.function.AggregationFunctionType;
+import org.apache.pinot.common.function.scalar.DataTypeConversionFunctions;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.core.common.BlockValSet;
+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.core.query.request.context.ExpressionContext;
+
+
+public class SumWithPrecisionAggregationFunction extends BaseSingleInputAggregationFunction<BigDecimal, BigDecimal> {
+ MathContext _mathContext = new MathContext(0);
+ Integer _scale = null;
+
+ public SumWithPrecisionAggregationFunction(ExpressionContext expression, Integer precision) {
Review comment:
if users dont use scale, will the results vary depending on the order of segment execution?
----------------------------------------------------------------
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.
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] [incubator-pinot] KKcorps commented on a change in pull request #6053: Add support for Decimal with Precision Sum aggregation
Posted by GitBox <gi...@apache.org>.
KKcorps commented on a change in pull request #6053:
URL: https://github.com/apache/incubator-pinot/pull/6053#discussion_r496624093
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
Review comment:
@siddharthteotia Tests have been added. Let me know if any changes are needed.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
##########
@@ -0,0 +1,76 @@
+/**
+ * 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.common.function.scalar;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.apache.pinot.common.function.annotations.ScalarFunction;
+
+
+public class DataTypeConversionFunctions {
+ private DataTypeConversionFunctions() {
+
+ }
+
+ @ScalarFunction
+ public static byte[] bigDecimalToBytes(BigDecimal number) {
+ int scale = number.scale();
+ BigInteger unscaled = number.unscaledValue();
+ byte[] value = unscaled.toByteArray();
+ byte[] bigDecimalBytesArray = new byte[value.length + 4];
+ for (int i = 0; i < 4; i++) {
+ bigDecimalBytesArray[i] = (byte) (scale >>> (8 * (3 - i)));
+ }
+ System.arraycopy(value, 0, bigDecimalBytesArray, 4, value.length);
+ return bigDecimalBytesArray;
+ }
+
+ @ScalarFunction
+ public static String bytesToBigDecimal(byte[] bytes) {
+ int scale = 0;
+ for (int i = 0; i < 4; i++) {
+ scale += (((int) bytes[i]) << (8 * (3 - i)));
+ }
+ byte[] vals = new byte[bytes.length - 4];
Review comment:
it is BIG endian
----------------------------------------------------------------
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.
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