This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 17ff44e127 Using isOptional instead of the deprecated
hasOptionalKeyword (#11682)
17ff44e127 is described below
commit 17ff44e127fcbd6eadbec51c7007411f38783ee1
Author: swaminathanmanish <[email protected]>
AuthorDate: Tue Sep 26 21:58:55 2023 -0700
Using isOptional instead of the deprecated hasOptionalKeyword (#11682)
---
.../inputformat/protobuf/ProtoBufRecordExtractor.java | 10 +++++++++-
.../protobuf/ProtoBufRecordExtractorTest.java | 17 +++++++++++++----
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git
a/pinot-plugins/pinot-input-format/pinot-protobuf/src/main/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractor.java
b/pinot-plugins/pinot-input-format/pinot-protobuf/src/main/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractor.java
index 6826fe7a82..5215ff94d0 100644
---
a/pinot-plugins/pinot-input-format/pinot-protobuf/src/main/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractor.java
+++
b/pinot-plugins/pinot-input-format/pinot-protobuf/src/main/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractor.java
@@ -53,8 +53,16 @@ public class ProtoBufRecordExtractor extends
BaseRecordExtractor<Message> {
}
}
+ /**
+ * For fields that are not set, we want to populate a null, instead of proto
default.
+ * @param fieldDescriptor
+ * @param message
+ */
private Object getFieldValue(Descriptors.FieldDescriptor fieldDescriptor,
Message message) {
- if (fieldDescriptor.isRepeated() || !fieldDescriptor.hasOptionalKeyword()
|| message.hasField(fieldDescriptor)) {
+ // Note w.r.t proto3 - If a field is not declared with optional keyword,
there's no way to distinguish
+ // if its explicitly set to a proto default or not been set at all i.e
hasField() returns false
+ // and we would use null.
+ if (fieldDescriptor.isRepeated() || !fieldDescriptor.isOptional() ||
message.hasField(fieldDescriptor)) {
return message.getField(fieldDescriptor);
} else {
return null;
diff --git
a/pinot-plugins/pinot-input-format/pinot-protobuf/src/test/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractorTest.java
b/pinot-plugins/pinot-input-format/pinot-protobuf/src/test/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractorTest.java
index 37738ef314..6380d1c5d0 100644
---
a/pinot-plugins/pinot-input-format/pinot-protobuf/src/test/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractorTest.java
+++
b/pinot-plugins/pinot-input-format/pinot-protobuf/src/test/java/org/apache/pinot/plugin/inputformat/protobuf/ProtoBufRecordExtractorTest.java
@@ -70,7 +70,7 @@ public class ProtoBufRecordExtractorTest extends
AbstractRecordExtractorTest {
@Override
protected List<Map<String, Object>> getInputRecords() {
- return Arrays.asList(createRecord1(), createRecord2());
+ return Arrays.asList(createExplicitOptionalDefaultRecord(),
createOptionalDefaultRecordWithNulls());
}
@Override
@@ -159,15 +159,21 @@ public class ProtoBufRecordExtractorTest extends
AbstractRecordExtractorTest {
.setNestedStringField((String)
nestedMessageFields.get(NESTED_STRING_FIELD)).build();
}
- private Map<String, Object> createRecord1() {
+ /**
+ * Test that verifies that optional fields with explicitly set default
protobuf values
+ * are preserved/not treated as nulls.
+ */
+ private Map<String, Object> createExplicitOptionalDefaultRecord() {
Map<String, Object> record = new HashMap<>();
record.put(STRING_FIELD, "hello");
record.put(INT_FIELD, 10);
record.put(LONG_FIELD, 100L);
record.put(DOUBLE_FIELD, 1.1);
record.put(FLOAT_FIELD, 2.2f);
- record.put(BOOL_FIELD, "false");
+ record.put(BOOL_FIELD, "true");
record.put(BYTES_FIELD, "hello world!".getBytes(UTF_8));
+ // These optional fields are explicitly set to proto defaults and expect
+ // to see these values preserved (not treated as null).
record.put(NULLABLE_STRING_FIELD, "");
record.put(NULLABLE_INT_FIELD, 0);
record.put(NULLABLE_LONG_FIELD, 0L);
@@ -188,7 +194,10 @@ public class ProtoBufRecordExtractorTest extends
AbstractRecordExtractorTest {
return record;
}
- private Map<String, Object> createRecord2() {
+ /**
+ * Test that verifies that optional fields not explicitly set are treated as
nulls.
+ */
+ private Map<String, Object> createOptionalDefaultRecordWithNulls() {
Map<String, Object> record = new HashMap<>();
record.put(STRING_FIELD, "world");
record.put(INT_FIELD, 20);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
