walterddr commented on code in PR #11824:
URL: https://github.com/apache/pinot/pull/11824#discussion_r1370292158
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/nullvalue/NullValueIndexTypeTest.java:
##########
@@ -0,0 +1,68 @@
+/**
Review Comment:
in addition, let's add some tests to the NullHandling.json test file to
validate the handling of null values are being considered
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeSystem.java:
##########
@@ -30,7 +31,8 @@
*/
public class TypeSystem extends RelDataTypeSystemImpl {
private static final int MAX_DECIMAL_SCALE = 1000;
- private static final int MAX_DECIMAL_PRECISION = 1000;
+ @VisibleForTesting
+ static final int MAX_DECIMAL_PRECISION = 1000;
Review Comment:
what's the reason behind this?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java:
##########
@@ -300,6 +303,27 @@ public void setTransformFunction(@Nullable String
transformFunction) {
_transformFunction = transformFunction;
}
+ /**
+ * Returns whether the column is nullable or not.
+ *
+ * This Java property itself is nullable. In case null is returned, actual
nullability depends on the value of
+ * {@link IndexingConfig#isNullHandlingEnabled()}.
+ *
+ * @return true if the field is nullable, false if it is not and null if
column level nullability should be delegated
+ * to {@link IndexingConfig#isNullHandlingEnabled()}.
+ */
+ @Nullable
+ public Boolean getNullable() {
Review Comment:
- should we default it to false unless otherwise specified (and thus no need
for non-primitive boolean)
- as long as the query flag `EnableNullHandling` overrides this setting we
should be good for v1 compatibility --> as in v2 we will no use this option
--
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]
