Jackie-Jiang commented on code in PR #8503:
URL: https://github.com/apache/pinot/pull/8503#discussion_r862142441
##########
pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java:
##########
@@ -38,10 +41,18 @@
public final class DefaultJsonPathEvaluator implements JsonPathEvaluator {
+ private static final ObjectMapper OBJECT_MAPPER_WITH_EXACT_BIG_DECIMAL = new
ObjectMapper()
Review Comment:
(minor) rename
```suggestion
private static final ObjectMapper OBJECT_MAPPER_WITH_BIG_DECIMAL = new
ObjectMapper()
```
##########
pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorService.java:
##########
@@ -104,13 +105,20 @@ private Comparator<Object[]>
getTypeCompatibleComparator(List<OrderByExpressionC
int numValuesToCompare = valueIndexList.size();
int[] valueIndices = new int[numValuesToCompare];
- boolean[] isNumber = new boolean[numValuesToCompare];
+ boolean[] useDoubleComparison = new boolean[numValuesToCompare];
+ boolean[] useBigDecimalComparison = new boolean[numValuesToCompare];
Review Comment:
We don't need to track this flag. `BigDecimal` can reuse the general
`Comparable` comparison
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunction.java:
##########
@@ -226,6 +246,46 @@ public double[] transformToDoubleValuesSV(ProjectionBlock
projectionBlock) {
return _doubleValuesSV;
}
+ @Override
+ public BigDecimal[] transformToBigDecimalValuesSV(ProjectionBlock
projectionBlock) {
+ int length = projectionBlock.getNumDocs();
+ if (_bigDecimalValuesSV == null || _bigDecimalValuesSV.length < length) {
+ _bigDecimalValuesSV = new BigDecimal[length];
+ }
+
+ Dictionary dictionary = getDictionary();
+ if (dictionary != null) {
+ int[] dictIds = transformToDictIdsSV(projectionBlock);
+ dictionary.readBigDecimalValues(dictIds, length, _bigDecimalValuesSV);
+ } else {
+ switch (getResultMetadata().getDataType().getStoredType()) {
+ case INT:
+ int[] intValues = transformToIntValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(intValues, _bigDecimalValuesSV, length);
+ break;
+ case LONG:
+ long[] longValues = transformToLongValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(longValues, _bigDecimalValuesSV, length);
+ break;
+ case FLOAT:
+ float[] floatValues = transformToFloatValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(floatValues, _bigDecimalValuesSV, length);
+ break;
+ case DOUBLE:
+ double[] doubleValues = transformToDoubleValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(doubleValues, _bigDecimalValuesSV, length);
+ break;
+ case STRING:
+ String[] stringValues = transformToStringValuesSV(projectionBlock);
+ ArrayCopyUtils.copy(stringValues, _bigDecimalValuesSV, length);
+ break;
Review Comment:
Consider adding `BYTES` here (as serialized `BigDecimal`)
##########
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java:
##########
@@ -366,15 +366,18 @@ public void runJob() {
case DOUBLE:
values[colId] = dataTable.getDouble(rowId, colId);
break;
+ case BIG_DECIMAL:
+ values[colId] = dataTable.getBigDecimal(rowId, colId);
+ break;
+ case OBJECT:
Review Comment:
Don't move the `OBJECT` case
##########
pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java:
##########
@@ -159,6 +161,14 @@ public void setColumn(int colId, double value) {
_currentRowDataByteBuffer.putDouble(value);
}
+ public void setColumn(int colId, BigDecimal value) throws IOException {
Review Comment:
(minor, code format) Please apply the [Pinot
Style](https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup?q=pinot+style#intellij)
and reformat
##########
pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java:
##########
@@ -38,10 +41,18 @@
public final class DefaultJsonPathEvaluator implements JsonPathEvaluator {
+ private static final ObjectMapper OBJECT_MAPPER_WITH_EXACT_BIG_DECIMAL = new
ObjectMapper()
+ .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);
+
private static final ParseContext JSON_PARSER_CONTEXT = JsonPath.using(
new Configuration.ConfigurationBuilder().jsonProvider(new
JacksonJsonProvider())
.mappingProvider(new
JacksonMappingProvider()).options(Option.SUPPRESS_EXCEPTIONS).build());
+ private static final ParseContext JSON_PARSER_CONTEXT_WITH_EXACT_BIG_DECIMAL
= JsonPath.using(
Review Comment:
(minor) rename
```suggestion
private static final ParseContext JSON_PARSER_CONTEXT_WITH_BIG_DECIMAL =
JsonPath.using(
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java:
##########
@@ -46,13 +49,21 @@
* jsonExtractScalar(jsonFieldName, 'jsonPath', 'resultsType')
* <code>jsonFieldName</code> is the Json String field/expression.
* <code>jsonPath</code> is a JsonPath expression which used to read from JSON
document
- * <code>results_type</code> refers to the results data type, could be INT,
LONG, FLOAT, DOUBLE, STRING, INT_ARRAY,
- * LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY.
+ * <code>results_type</code> refers to the results data type, could be INT,
LONG, FLOAT, DOUBLE, BIG_DECIMAL, STRING,
+ * INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY.
*
*/
public class JsonExtractScalarTransformFunction extends BaseTransformFunction {
public static final String FUNCTION_NAME = "jsonExtractScalar";
+ private static final ObjectMapper OBJECT_MAPPER_WITH_EXACT_BIG_DECIMAL = new
ObjectMapper()
Review Comment:
(minor) rename
```suggestion
private static final ObjectMapper OBJECT_MAPPER_WITH_BIG_DECIMAL = new
ObjectMapper()
```
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/ForwardIndexReader.java:
##########
@@ -258,6 +259,47 @@ default void readValuesSV(int[] docIds, int length,
double[] values, T context)
}
}
+ /**
+ * Fills the values
+ * @param docIds Array containing the document ids to read
+ * @param length Number of values to read
+ * @param values Values to fill
+ * @param context Reader context
+ */
+ default void readValuesSV(int[] docIds, int length, BigDecimal[] values, T
context) {
+ // todo(nhejazi): add raw index support to the BIG_DECIMAL type. In most
of the cases, it will be more efficient
+ // to store big decimal as raw.
+ switch (getValueType()) {
+ case INT:
+ for (int i = 0; i < length; i++) {
+ values[i] = BigDecimal.valueOf(getInt(docIds[i], context));
+ }
+ break;
+ case LONG:
+ for (int i = 0; i < length; i++) {
+ values[i] = BigDecimal.valueOf(getLong(docIds[i], context));
+ }
+ break;
+ case FLOAT:
+ for (int i = 0; i < length; i++) {
+ values[i] = BigDecimal.valueOf(getFloat(docIds[i], context));
+ }
+ break;
+ case DOUBLE:
+ for (int i = 0; i < length; i++) {
+ values[i] = BigDecimal.valueOf(getDouble(docIds[i], context));
+ }
+ break;
+ case STRING:
+ for (int i = 0; i < length; i++) {
+ values[i] = new BigDecimal(getString(docIds[i], context));
+ }
+ break;
Review Comment:
Consider supporting `BYTES` (serialized BigDecimal)
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java:
##########
@@ -46,13 +49,21 @@
* jsonExtractScalar(jsonFieldName, 'jsonPath', 'resultsType')
* <code>jsonFieldName</code> is the Json String field/expression.
* <code>jsonPath</code> is a JsonPath expression which used to read from JSON
document
- * <code>results_type</code> refers to the results data type, could be INT,
LONG, FLOAT, DOUBLE, STRING, INT_ARRAY,
- * LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY.
+ * <code>results_type</code> refers to the results data type, could be INT,
LONG, FLOAT, DOUBLE, BIG_DECIMAL, STRING,
+ * INT_ARRAY, LONG_ARRAY, FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY.
*
*/
public class JsonExtractScalarTransformFunction extends BaseTransformFunction {
public static final String FUNCTION_NAME = "jsonExtractScalar";
+ private static final ObjectMapper OBJECT_MAPPER_WITH_EXACT_BIG_DECIMAL = new
ObjectMapper()
+ .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);
+
+ private static final ParseContext JSON_PARSER_CONTEXT_WITH_EXACT_BIG_DECIMAL
= JsonPath.using(
Review Comment:
(minor) rename
```suggestion
private static final ParseContext JSON_PARSER_CONTEXT_WITH_BIG_DECIMAL =
JsonPath.using(
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
