clintropolis commented on code in PR #13712:
URL: https://github.com/apache/druid/pull/13712#discussion_r1092682707
##########
core/src/test/java/org/apache/druid/data/input/impl/JsonLineReaderTest.java:
##########
@@ -141,7 +141,7 @@ public void testParseRowWithConditional() throws IOException
while (iterator.hasNext()) {
final InputRow row = iterator.next();
Assert.assertEquals("test",
Iterables.getOnlyElement(row.getDimension("bar")));
- Assert.assertEquals(Collections.emptyList(), row.getDimension("foo"));
+ Assert.assertEquals("null", row.getDimension("foo").get(0));
Review Comment:
hmm, why would this change happen? there seems like no 'foo' in the row, a
`null` should be turned into an empty list from `getDimension`, not a list with
a string `"null"` in it
##########
extensions-core/avro-extensions/src/test/java/org/apache/druid/data/input/avro/AvroFlattenerMakerTest.java:
##########
@@ -210,6 +211,45 @@ public void testDiscovery()
);
}
+
+ @Test
+ public void testNullsInStringArray()
+ {
+ final AvroFlattenerMaker flattenerNested = new AvroFlattenerMaker(false,
false, true, true);
+
+ SomeAvroDatum input = AvroStreamInputRowParserTest.buildSomeAvroDatum();
+
+ Assert.assertEquals(
+ ImmutableSet.of(
+ "someStringValueMap",
+ "someOtherId",
+ "someStringArray",
+ "someIntArray",
+ "someFloat",
+ "isValid",
+ "someIntValueMap",
+ "eventType",
+ "someFixed",
+ "someBytes",
+ "someRecord",
+ "someMultiMemberUnion",
+ "someNull",
+ "someRecordArray",
+ "someUnion",
+ "id",
+ "someEnum",
+ "someLong",
+ "someInt",
+ "timestamp"
+ ),
+ ImmutableSet.copyOf(flattenerNested.discoverRootFields(input))
+ );
+
+ ArrayList<Object> results = (ArrayList<Object>)
flattenerNested.getRootField(input, "someStringArray");
+ // 4 strings a 1 null for a total of 5
+ Assert.assertEquals(5, results.size());
Review Comment:
similar comment about maybe nice to have a flatten expression that extracts
a null from the array
##########
core/src/test/java/org/apache/druid/data/input/impl/JSONParseSpecTest.java:
##########
@@ -113,6 +113,40 @@ public void testParseRowWithConditional()
Assert.assertEquals(expected, parsedRow);
}
+ @Test
+ public void testParseRowWithNullsInMVD()
+ {
+ final JSONParseSpec parseSpec = new JSONParseSpec(
+ new TimestampSpec("timestamp", "iso", null),
+ new
DimensionsSpec(DimensionsSpec.getDefaultSchemas(ImmutableList.of("foo"))),
+ new JSONPathSpec(
+ true,
+ ImmutableList.of(
+ // https://github.com/apache/druid/issues/6653 $.x.y.z where y
is missing
+ new JSONPathFieldSpec(JSONPathFieldType.PATH, "foo",
"$.baz.[?(@.maybe_object)].maybe_object"),
+ // https://github.com/apache/druid/issues/6653 $.x.y.z where y
is null
+ new JSONPathFieldSpec(JSONPathFieldType.PATH, "nullFoo",
"$.nullFoo.[?(@.value)].foo"),
+ new JSONPathFieldSpec(JSONPathFieldType.PATH, "baz", "$.baz"),
+ new JSONPathFieldSpec(JSONPathFieldType.PATH, "bar",
"$.[?(@.something_else)].something_else.foo")
Review Comment:
most of these paths don't really seem meaningful for this test
I think something like `$.baz[1]` would be useful to extract a null element
from the array.
if you're feeling ambitious, it might also be useful to have tests on arrays
of objects and paths to extract things from them.
##########
core/src/main/java/org/apache/druid/java/util/common/parsers/JSONFlattenerMaker.java:
##########
@@ -179,12 +179,13 @@ public static Object convertJsonNode(JsonNode val,
CharsetEncoder enc)
return ((BinaryNode) val).binaryValue();
}
+
if (val.isArray()) {
List<Object> newList = new ArrayList<>();
for (JsonNode entry : val) {
- if (!entry.isNull()) {
- newList.add(convertJsonNode(entry, enc));
- }
+ // process nulls as is
+ // convertJsonNode will return null if the entry is null
Review Comment:
nit: this comment doesn't really add anything :p
--
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]
