[GitHub] [druid] clintropolis commented on a change in pull request #9999: Optimize protobuf parsing for flatten data

[GitBox <gi...@apache.org>]

clintropolis commented on a change in pull request #9999:
URL: https://github.com/apache/druid/pull/9999#discussion_r444740588



##########
File path: extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtobufInputRowParserTest.java
##########
@@ -177,6 +191,45 @@ public void testParse() throws Exception
     Assert.assertEquals(816.0F, row.getMetric("someLongColumn").floatValue(), 0.0);
   }
 
+  @Test
+  public void testParseFlattenData() throws Exception
+  {
+    //configure parser with desc file
+    ProtobufInputRowParser parser = new ProtobufInputRowParser(flattenParseSpec, "prototest.desc", "ProtoTestEvent");
+
+    //create binary of proto test event
+    DateTime dateTime = new DateTime(2012, 7, 12, 9, 30, ISOChronology.getInstanceUTC());
+    ProtoTestEventWrapper.ProtoTestEvent event = ProtoTestEventWrapper.ProtoTestEvent.newBuilder()
+            .setDescription("description")
+            .setEventType(ProtoTestEventWrapper.ProtoTestEvent.EventCategory.CATEGORY_ONE)
+            .setId(4711L)
+            .setIsValid(true)
+            .setSomeOtherId(4712)
+            .setTimestamp(dateTime.toString())
+            .setSomeFloatColumn(47.11F)
+            .setSomeIntColumn(815)
+            .setSomeLongColumn(816L)
+            .build();
+
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    event.writeTo(out);
+
+    InputRow row = parser.parseBatch(ByteBuffer.wrap(out.toByteArray())).get(0);
+    System.out.println(row);

Review comment:
       nit: unnecessary println, though there are others in this test file prior to this change so no worries

##########
File path: extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtobufInputRowParserTest.java
##########
@@ -76,6 +77,19 @@ public void setUp()
         null
     );
 
+    flattenParseSpec = new JSONParseSpec(

Review comment:
       nit: should this be named `flatParseSpec` since it is for flat data and _will not_ flatten the data since it has a `null` flattenSpec?

##########
File path: extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtobufInputRowParserTest.java
##########
@@ -177,6 +191,45 @@ public void testParse() throws Exception
     Assert.assertEquals(816.0F, row.getMetric("someLongColumn").floatValue(), 0.0);
   }
 
+  @Test
+  public void testParseFlattenData() throws Exception

Review comment:
       nit: same suggestion about naming, should this be `testParseFlatData` since it isn't actively flattening nested data?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org