amrishlal commented on a change in pull request #6719:
URL: https://github.com/apache/incubator-pinot/pull/6719#discussion_r612097293
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
##########
@@ -215,7 +236,34 @@ public int hashCode() {
}
public enum ColumnDataType {
- INT, LONG, FLOAT, DOUBLE, STRING, BYTES, OBJECT, INT_ARRAY, LONG_ARRAY,
FLOAT_ARRAY, DOUBLE_ARRAY, STRING_ARRAY;
+ INT,
+ LONG,
+ FLOAT,
+ DOUBLE,
+ BOOLEAN /* Stored as INT */,
Review comment:
Should `BOOLEAN` should be stored as `BYTE` to save space?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java
##########
@@ -92,6 +115,16 @@ public double toDouble(Object value) {
return ((Byte) value).doubleValue();
}
+ @Override
+ public boolean toBoolean(Object value) {
+ return (Byte) value > 0;
Review comment:
Can we replace this by `return (Byte) value != 0`? I think the common
convention (at least in c/c++ and a few other languages) is to treat zero as
false and everything else as true. Same with other data types (character,
integer, long, float) toBoolean function.
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java
##########
@@ -315,6 +408,58 @@ public Double convert(Object value, PinotDataType
sourceType) {
}
},
+ TIMESTAMP {
+ @Override
+ public int toInt(Object value) {
+ throw new UnsupportedOperationException("Cannot convert value from
TIMESTAMP to INTEGER");
+ }
+
+ @Override
+ public long toLong(Object value) {
+ return ((Timestamp) value).getTime();
+ }
+
+ @Override
+ public float toFloat(Object value) {
+ throw new UnsupportedOperationException("Cannot convert value from
TIMESTAMP to FLOAT");
+ }
+
+ @Override
+ public double toDouble(Object value) {
+ return ((Timestamp) value).getTime();
+ }
+
+ @Override
+ public boolean toBoolean(Object value) {
+ throw new UnsupportedOperationException("Cannot convert value from
TIMESTAMP to BOOLEAN");
Review comment:
I would have done zero as `false` and all other timestamp values as
`true`. This convention generalizes to work with everything and type conversion
becomes simple :-)
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java
##########
@@ -375,6 +530,16 @@ public double toDouble(Object value) {
throw new UnsupportedOperationException("Cannot convert value from BYTES
to DOUBLE");
}
+ @Override
+ public boolean toBoolean(Object value) {
+ throw new UnsupportedOperationException("Cannot convert value from BYTES
to BOOLEAN");
Review comment:
If BYTES is not 0, it is true; otherwise, false?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/PinotDataType.java
##########
@@ -412,6 +577,16 @@ public double toDouble(Object value) {
return ((Number) value).doubleValue();
}
+ @Override
+ public boolean toBoolean(Object value) {
+ return ((Number) value).intValue() > 0;
Review comment:
Can object be null? Would null Object be considered true or false? What
if we add NULL value support later?
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
