snuyanzin commented on code in PR #26217: URL: https://github.com/apache/flink/pull/26217#discussion_r1973343687
########## flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/JsonSmileSerdeUtil.java: ########## @@ -71,51 +73,71 @@ import java.io.IOException; import java.util.Optional; -/** A utility class that provide abilities for JSON serialization and deserialization. */ +/** A utility class that provide abilities for JSON and Smile serialization and deserialization. */ @Internal -public class JsonSerdeUtil { +public class JsonSmileSerdeUtil { /** * Object mapper shared instance to serialize and deserialize the plan. Note that creating and * copying of object mappers is expensive and should be avoided. * * <p>This is not exposed to avoid bad usages, like adding new modules. If you need to read and - * write json persisted plans, use {@link #createObjectWriter(SerdeContext)} and {@link - * #createObjectReader(SerdeContext)}. + * write json or smile persisted plans, use {@link #createJsonObjectWriter(SerdeContext)} and + * {@link #createJsonObjectReader(SerdeContext)}. */ - private static final ObjectMapper OBJECT_MAPPER_INSTANCE; + private static final ObjectMapper JSON_OBJECT_MAPPER_INSTANCE; + + private static final ObjectMapper SMILE_OBJECT_MAPPER_INSTANCE = + JacksonMapperFactory.createObjectMapper(new SmileFactory()) + .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) + .registerModule(createFlinkTableJacksonModule()); Review Comment: Added `MapperFeature.USE_GETTERS_AS_SETTERS` for consistency. about `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS`, IIRC the only reason to have it is our tests against actual plans and the fact that it looks like in jdk (between 18 and 21) something has been changed in hashcode and same tests started to fail with jdk21. More info at https://github.com/apache/flink/pull/23562 and FLINK-33334. So this is not the case for `Smile`, so we could omit it -- 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