[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r325371131
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java
##
@@ -574,8 +575,14 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
if (defaultNullValue == null) {
defaultNullValue = fieldSpec.getDefaultNullValue();
}
-properties.setProperty(V1Constants.MetadataKeys.Column.getKeyFor(column,
DEFAULT_NULL_VALUE),
-String.valueOf(defaultNullValue));
+if (fieldSpec.getDataType() == FieldSpec.DataType.BYTES &&
defaultNullValue instanceof byte[]) {
Review comment:
Removed.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r325371081
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java
##
@@ -574,8 +575,14 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
if (defaultNullValue == null) {
Review comment:
Correct, we have default null value for all kinds of types in FieldSpec.
Removed.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r324043427
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
##
@@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
dictionaryElementSize =
StringUtil.encodeUtf8(stringDefaultValue).length;
sortedArray = new String[]{stringDefaultValue};
break;
+ case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
+sortedArray = new ByteArray[] {new ByteArray((byte[]) defaultValue)};
Review comment:
Yes, the default value is decoded in `convert` method in FieldSpec.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r323981558
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
##
@@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
dictionaryElementSize =
StringUtil.encodeUtf8(stringDefaultValue).length;
sortedArray = new String[]{stringDefaultValue};
break;
+ case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
Review comment:
It doesn't have to be >=1. The following is the default value of byte column
in `FieldSpec` class which length is 0.
```
private static final byte[] NULL_BYTE_ARRAY_VALUE = new byte[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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r323980970
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
##
@@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
dictionaryElementSize =
StringUtil.encodeUtf8(stringDefaultValue).length;
sortedArray = new String[]{stringDefaultValue};
break;
+ case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
+sortedArray = new ByteArray[] {new ByteArray((byte[]) defaultValue)};
Review comment:
Doc updated.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321940046
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
##
@@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
dictionaryElementSize =
StringUtil.encodeUtf8(stringDefaultValue).length;
sortedArray = new String[]{stringDefaultValue};
break;
+ case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
+sortedArray = new ByteArray[] {new ByteArray((byte[]) defaultValue)};
Review comment:
Actually the value is already of byte array type. There is no need to decode
it again. We can document what/how to put a default value for byte column.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321940046
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
##
@@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
dictionaryElementSize =
StringUtil.encodeUtf8(stringDefaultValue).length;
sortedArray = new String[]{stringDefaultValue};
break;
+ case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
+sortedArray = new ByteArray[] {new ByteArray((byte[]) defaultValue)};
Review comment:
Actually the value is already of byte array type. We can document what/how
to put a default value for byte column.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321939628
##
File path: pinot-core/src/test/resources/data/newColumnsSchema3.json
##
@@ -16,6 +16,11 @@
{
"dataType": "DOUBLE",
"name": "newDoubleMetric"
+},
+{
+ "dataType": "BYTES",
+ "defaultNullValue": "000800ac",
Review comment:
Yes, this is helix string of the byte array.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321384433
##
File path: pinot-core/src/test/resources/data/newColumnsSchema3.json
##
@@ -16,6 +16,11 @@
{
"dataType": "DOUBLE",
"name": "newDoubleMetric"
+},
+{
+ "dataType": "BYTES",
+ "defaultNullValue": "000800ac",
Review comment:
This is one value of hyperLogLog
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4583: Support default value for Byte column
jackjlli commented on a change in pull request #4583: Support default value for
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r320992567
##
File path:
pinot-common/src/test/java/org/apache/pinot/common/data/FieldSpecTest.java
##
@@ -117,6 +117,17 @@ public void testFieldSpec() {
Assert.assertEquals(fieldSpec1.toString(), fieldSpec2.toString());
Assert.assertEquals(fieldSpec1.hashCode(), fieldSpec2.hashCode());
Assert.assertEquals(fieldSpec1.getDefaultNullValue(), 1L);
+
+// Metric field with default null value for byte column.
+fieldSpec1 = new MetricFieldSpec();
+fieldSpec1.setName("btyeMetric");
Review comment:
good catch
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
