wenlong88 commented on a change in pull request #15219:
URL: https://github.com/apache/flink/pull/15219#discussion_r594841777
##########
File path:
flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/serde/LogicalTypeSerdeTest.java
##########
@@ -105,7 +105,7 @@ public void testLogicalTypeSerde() throws IOException {
}
String json = writer.toString();
LogicalType actual = mapper.readValue(json, LogicalType.class);
- assertEquals(logicalType, actual);
+ assertEquals(logicalType.asSerializableString(),
actual.asSerializableString());
Review comment:
since the asSerializableString can not represent all use case, we can
not compare the result using this method, maybe just keep the equals compare
and add add some specical cases
##########
File path:
flink-table/flink-table-planner-blink/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/JsonSerdeCoverageTest.java
##########
@@ -72,8 +72,7 @@
"StreamExecMultipleInput",
"StreamExecMatch",
"StreamExecUnion",
- "StreamExecValues",
- "StreamExecDeduplicate");
Review comment:
cannot remove here
##########
File path:
flink-table/flink-table-planner-blink/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/LogicalTypeJsonSerializer.java
##########
@@ -102,15 +122,91 @@ public void serialize(
} else if (logicalType instanceof DistinctType) {
// DistinctType does not full support `asSerializableString`
serialize((DistinctType) logicalType, jsonGenerator);
+ } else if (logicalType instanceof TimestampType) {
+ // TimestampType does not consider `TimestampKind`
+ serialize((TimestampType) logicalType, jsonGenerator);
+ } else if (logicalType instanceof ZonedTimestampType) {
+ // ZonedTimestampType does not consider `TimestampKind`
+ serialize((ZonedTimestampType) logicalType, jsonGenerator);
+ } else if (logicalType instanceof LocalZonedTimestampType) {
+ // LocalZonedTimestampType does not consider `TimestampKind`
+ serialize((LocalZonedTimestampType) logicalType, jsonGenerator);
} else if (logicalType instanceof UnresolvedUserDefinedType) {
throw new TableException(
"Can not serialize an UnresolvedUserDefinedType instance.
\n"
+ "It needs to be resolved into a proper
user-defined type.\"");
+ } else if (logicalType instanceof RowType) {
+ serializeRowType((RowType) logicalType, jsonGenerator,
serializerProvider);
+ } else if (logicalType instanceof MapType) {
+ serializeMapType((MapType) logicalType, jsonGenerator,
serializerProvider);
+ } else if (logicalType instanceof ArrayType) {
+ serializeArrayType((ArrayType) logicalType, jsonGenerator,
serializerProvider);
+ } else if (logicalType instanceof MultisetType) {
+ serializeMultisetType((MultisetType) logicalType, jsonGenerator,
serializerProvider);
} else {
jsonGenerator.writeObject(logicalType.asSerializableString());
}
}
+ private void serializeRowType(
+ RowType rowType, JsonGenerator jsonGenerator, SerializerProvider
serializerProvider)
+ throws IOException {
+ jsonGenerator.writeStartObject();
+ jsonGenerator.writeStringField(FIELD_NAME_TYPE_NAME,
rowType.getTypeRoot().name());
+ jsonGenerator.writeBooleanField(FIELD_NAME_NULLABLE,
rowType.isNullable());
+ List<RowType.RowField> fields = rowType.getFields();
+ jsonGenerator.writeArrayFieldStart(FIELD_NAME_FIELDS);
+ for (RowType.RowField rowField : fields) {
+ jsonGenerator.writeStartObject();
+ jsonGenerator.writeFieldName(rowField.getName());
+ serialize(rowField.getType(), jsonGenerator, serializerProvider);
+ if (rowField.getDescription().isPresent()) {
+ jsonGenerator.writeStringField(
Review comment:
I don't think we need to serialize description here, if we need, we may
need one field.
----------------------------------------------------------------
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
