dawidwys commented on code in PR #26217: URL: https://github.com/apache/flink/pull/26217#discussion_r1973158894
########## 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: Could we initialize the two mappers the same way? Either in the `static {}` block or in the fields section? Moreover, don't we need `MapperFeature.USE_GETTERS_AS_SETTERS` and `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS`? ########## 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()); static { - OBJECT_MAPPER_INSTANCE = JacksonMapperFactory.createObjectMapper(); + JSON_OBJECT_MAPPER_INSTANCE = JacksonMapperFactory.createObjectMapper(); - OBJECT_MAPPER_INSTANCE.setTypeFactory( + JSON_OBJECT_MAPPER_INSTANCE.setTypeFactory( // Make sure to register the classloader of the planner - OBJECT_MAPPER_INSTANCE + JSON_OBJECT_MAPPER_INSTANCE .getTypeFactory() - .withClassLoader(JsonSerdeUtil.class.getClassLoader())); - OBJECT_MAPPER_INSTANCE.configure(MapperFeature.USE_GETTERS_AS_SETTERS, false); - OBJECT_MAPPER_INSTANCE.configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true); - OBJECT_MAPPER_INSTANCE.registerModule(createFlinkTableJacksonModule()); + .withClassLoader(JsonSmileSerdeUtil.class.getClassLoader())); + JSON_OBJECT_MAPPER_INSTANCE.configure(MapperFeature.USE_GETTERS_AS_SETTERS, false); + JSON_OBJECT_MAPPER_INSTANCE.configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true); + JSON_OBJECT_MAPPER_INSTANCE.registerModule(createFlinkTableJacksonModule()); } - public static ObjectReader createObjectReader(SerdeContext serdeContext) { - return OBJECT_MAPPER_INSTANCE + public static ObjectReader createJsonObjectReader(SerdeContext serdeContext) { + return JSON_OBJECT_MAPPER_INSTANCE .reader() .withAttribute(SerdeContext.SERDE_CONTEXT_KEY, serdeContext) .with(defaultInjectedValues()); } - public static ObjectWriter createObjectWriter(SerdeContext serdeContext) { - return OBJECT_MAPPER_INSTANCE + public static ObjectWriter createJsonObjectWriter(SerdeContext serdeContext) { + return JSON_OBJECT_MAPPER_INSTANCE .writer() .withAttribute(SerdeContext.SERDE_CONTEXT_KEY, serdeContext); } + public static ObjectWriter createSmileObjectWriter(SerdeContext serdeContext) { + return SMILE_OBJECT_MAPPER_INSTANCE + .writer() + .withAttribute(SerdeContext.SERDE_CONTEXT_KEY, serdeContext) + .with(SmileGenerator.Feature.CHECK_SHARED_STRING_VALUES); + } + + public static ObjectReader createSmileObjectReader(SerdeContext serdeContext) { + return SMILE_OBJECT_MAPPER_INSTANCE + .reader() + .withAttribute(SerdeContext.SERDE_CONTEXT_KEY, serdeContext) + .with(SmileGenerator.Feature.CHECK_SHARED_STRING_VALUES) + .with(defaultInjectedValues()); + } + private static InjectableValues defaultInjectedValues() { return new InjectableValues.Std().addValue("isDeserialize", true); } - private static Module createFlinkTableJacksonModule() { + static Module createFlinkTableJacksonModule() { Review Comment: Why did you have to change the scope? -- 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