Jackie-Jiang commented on code in PR #8468:
URL: https://github.com/apache/pinot/pull/8468#discussion_r844235906
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -250,6 +251,7 @@ public int hashCode() {
FLOAT,
DOUBLE,
BOOLEAN /* Stored as INT */,
+ BIG_DECIMAL /* Stored as BYTES */,
Review Comment:
(minor) Move it in front of BOOLEAN for consistency (Also keeping number
types together)
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionUtils.java:
##########
@@ -42,6 +43,7 @@ private FunctionUtils() {
put(double.class, PinotDataType.DOUBLE);
put(Double.class, PinotDataType.DOUBLE);
put(boolean.class, PinotDataType.BOOLEAN);
+ put(BigDecimal.class, PinotDataType.BIG_DECIMAL);
Review Comment:
(minor) Move it in front of boolean for consistency
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataTable.java:
##########
@@ -56,6 +57,8 @@
double getDouble(int rowId, int colId);
+ BigDecimal getBigDecimal(int rowId, int colId);
Review Comment:
For data access layer, we should not introduce the `BigDecimal` type, but
use the stored type (see how `Timestamp` is handled)
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -136,6 +143,11 @@ public double toDouble(Object value) {
return ((Number) value).doubleValue();
}
+ @Override
+ public BigDecimal toBigDecimal(Object value) {
+ return new BigDecimal(toInt(value));
Review Comment:
Should we consider using `BigDecimal.valueOf()`? Same for other places
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -1281,16 +1419,20 @@ public static PinotDataType
getPinotDataTypeForIngestion(FieldSpec fieldSpec) {
return fieldSpec.isSingleValueField() ? FLOAT : FLOAT_ARRAY;
case DOUBLE:
return fieldSpec.isSingleValueField() ? DOUBLE : DOUBLE_ARRAY;
+ case BIG_DECIMAL:
+ if (fieldSpec.isSingleValueField()) {
+ return BIG_DECIMAL;
+ }
+ throw new UnsupportedOperationException("Multi-value BigDecimal data
type is not currently supported");
Review Comment:
(minor) Let's keep the exception type consistent
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java:
##########
@@ -1192,6 +1327,9 @@ public static PinotDataType getSingleValueType(Class<?>
cls) {
if (cls == String.class) {
return STRING;
}
+ if (cls == BigDecimal.class) {
Review Comment:
(minor) Move it after `Timestamp` as it should be rarely used as the input
object
##########
pinot-core/src/main/java/org/apache/pinot/core/common/BlockValSet.java:
##########
@@ -84,6 +85,13 @@
*/
double[] getDoubleValuesSV();
+ /**
+ * Returns the BigDecimal[] values for a single-valued column.
+ *
+ * @return Array of BigDecimal[] values
+ */
+ BigDecimal[] getBigDecimalValuesSV();
Review Comment:
This API is not required as we don't want to make `BigDecimal` as a storage
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
