[ https://issues.apache.org/jira/browse/FLINK-31917?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17719123#comment-17719123 ]
Shengkai Fang commented on FLINK-31917: --------------------------------------- Merged into master: 333e023196d90d265c286632f8c01c41b8911ef8 > Loss of Idempotence in JsonSerDe Round Trip for AggregateCall and RexNode > ------------------------------------------------------------------------- > > Key: FLINK-31917 > URL: https://issues.apache.org/jira/browse/FLINK-31917 > Project: Flink > Issue Type: Bug > Components: Table SQL / Planner > Affects Versions: 1.18.0 > Reporter: Jane Chan > Assignee: Jane Chan > Priority: Major > Labels: pull-request-available > Fix For: 1.18.0 > > > JsonSerDeTestUtil#testJsonRoundTrip only checks the equality between spec and > deserialized object. Some corner cases are detected when serializing the > deserialized object again. > {code:java} > static <T> T testJsonRoundTrip(SerdeContext serdeContext, T spec, Class<T> > clazz) > throws IOException { > String actualJson = toJson(serdeContext, spec); > T actual = toObject(serdeContext, actualJson, clazz); > assertThat(actual).isEqualTo(spec); > assertThat(actualJson).isEqualTo(toJson(serdeContext, actual)); // > this will eval some corner cases > return actual; > } > {code} > The discovered corner cases are listed as follows. > h5. 1. SerDe for AggregateCall > When deserializing the aggregate call, we should check the JsonNodeType to > avoid converting null to "null" string. > [https://github.com/apache/flink/blob/f9b3e0b7bc0432001b4a197539a0712b16e0b33b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/AggregateCallJsonDeserializer.java#L64] > h5. Suggested Fix > {code:java} > JsonNode nameNode = jsonNode.required(FIELD_NAME_NAME); > final String name = JsonNodeType.NULL ? null : nameNode.asText(); > {code} > h5. 2. SerDe for RexNode > RexNodeJsonSerdeTest#testSystemFunction should create the temporary system > function instead of the temporary catalog function. > [https://github.com/apache/flink/blob/f9b3e0b7bc0432001b4a197539a0712b16e0b33b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/serde/RexNodeJsonSerdeTest.java#L209] > h5. Suggested Fix > Use functionCatalog#registerTemporarySystemFunction to test. > h5. 3. About RexLiteral type > RexNodeJsonSerdeTest#testRexNodeSerde has a test spec as follows > {code:java} > //This will create the literal with DOUBLE as the literal type, and DECIMAL > as the broad type of this literal. You can refer to Calcite for more details > rexBuilder.makeExactLiteral(BigDecimal.valueOf(Double.MAX_VALUE), > FACTORY.createSqlType(SqlTypeName.DOUBLE)) > {code} > The RexNodeJsonSerializer uses `typeName`(which is DECIMAL) as the literal's > type, as a result, the rel data type is serialized as double, but the value > is serialized as a string (in case lost the precision) > [https://github.com/apache/flink/blob/f9b3e0b7bc0432001b4a197539a0712b16e0b33b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/RexNodeJsonSerializer.java#L197] > And then, during the deserialization, according to the JSON, the deserialized > literal will assign DOUBLE as the literal type and the broad type of the > literal. > This will cause the comparison failure > {code:java} > expected: {"kind": "LITERAL", "value": "1.7976931348623157E+308"} > actual: {"kind": "LITERAL", "value": 1.7976931348623157E+308} > {code} > h5. Suggested Fix > SARG is a special case and can be coped first, and for the rest type, we can > use literal.getType().getSqlTypeName() instead of literal.getTypeName(). > {code:java} > // first cope with SARG type > if (literal.getTypeName() == SARG) { > serializeSargValue( > (Sarg<?>) value, literal.getType().getSqlTypeName(), gen, > serializerProvider); > } else { > serializeLiteralValue( > value, > literal.getType().getSqlTypeName(), > gen, > serializerProvider); > } > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)