maosuhan commented on code in PR #21436: URL: https://github.com/apache/flink/pull/21436#discussion_r1037743338
########## flink-formats/flink-protobuf/src/test/java/org/apache/flink/formats/protobuf/Pb3ToRowTest.java: ########## @@ -85,6 +88,8 @@ public void testDeserialization() throws Exception { assertEquals(1, rowData.getInt(0)); assertEquals(2L, rowData.getLong(1)); + + assertEquals(ts.getNanos(), row.getTimestamp(11, 3).getNanoOfMillisecond()); Review Comment: I'm curious how does Timestamp work because I did not define Timestamp type for flink. I guess Timestamp is a message type in protobuf, how come it will convert to flink Timestamp automatically? ########## flink-formats/flink-protobuf/src/main/java/org/apache/flink/formats/protobuf/util/PbFormatUtils.java: ########## @@ -34,6 +35,15 @@ public static String getFullJavaName(Descriptors.Descriptor descriptor, String o String parentJavaFullName = getFullJavaName(descriptor.getContainingType(), outerProtoName); return parentJavaFullName + "." + descriptor.getName(); + } else if (descriptor.getFullName().contains("google")) { Review Comment: @hdulay The compilation error is caused by a third-party import, right? Could you make it more general to adapt to all the import cases? ########## flink-formats/flink-protobuf/src/test/proto/test_pb3.proto: ########## @@ -33,6 +34,7 @@ message Pb3Test { bytes i = 9; map<string, string> map1 = 10; map<string, InnerMessageTest> map2 = 11; + google.protobuf.Timestamp ts = 12; Review Comment: Also test in ProtobufSQLITCaseTest for end to end correctness. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org