[
https://issues.apache.org/jira/browse/FLINK-31917?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jane Chan updated FLINK-31917:
------------------------------
Description:
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}
was:
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. 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}
expected: {"kind": "LITERAL", "value": "1.7976931348623157E+308"}
actual: {"kind": "LITERAL", "value": 1.7976931348623157E+308}
{code}
> 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
> Priority: Major
> 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)
