From 3aa3d4bcf196586f7e3b78bc368dbd715cb86139 Mon Sep 17 00:00:00 2001 From: "Mateusz \"Serafin\" Gajewski" Date: Fri, 22 Nov 2024 13:29:33 +0100 Subject: [PATCH] Decouple Sql types from JSON serialization The new JSON serialization is not using ObjectMapper to serialize these values anymore. We want to decouple SPI types from JSON representation to be able to introduce alternative encoding formats. --- core/trino-spi/pom.xml | 48 +++++++++++++++++++ .../main/java/io/trino/spi/type/SqlDate.java | 3 -- .../java/io/trino/spi/type/SqlDecimal.java | 3 -- .../main/java/io/trino/spi/type/SqlTime.java | 3 -- .../trino/spi/type/SqlTimeWithTimeZone.java | 3 -- .../java/io/trino/spi/type/SqlTimestamp.java | 3 -- .../spi/type/SqlTimestampWithTimeZone.java | 3 -- .../trino/plugin/base/util/JsonTypeUtil.java | 31 +++++++++++- .../plugin/postgresql/PostgreSqlClient.java | 8 +++- 9 files changed, 83 insertions(+), 22 deletions(-) diff --git a/core/trino-spi/pom.xml b/core/trino-spi/pom.xml index 704f9e702b47..e6358e816586 100644 --- a/core/trino-spi/pom.xml +++ b/core/trino-spi/pom.xml @@ -226,6 +226,54 @@ @com.fasterxml.jackson.annotation.JsonValue On-the-wire representation shouldn't rely on the Jackson format + + true + java.annotation.removed + method java.lang.String io.trino.spi.type.SqlDate::toString() + method java.lang.String io.trino.spi.type.SqlDate::toString() + @com.fasterxml.jackson.annotation.JsonValue + On-the-wire representation shouldn't rely on the Jackson format + + + true + java.annotation.removed + method java.lang.String io.trino.spi.type.SqlDecimal::toString() + method java.lang.String io.trino.spi.type.SqlDecimal::toString() + @com.fasterxml.jackson.annotation.JsonValue + On-the-wire representation shouldn't rely on the Jackson format + + + true + java.annotation.removed + method java.lang.String io.trino.spi.type.SqlTime::toString() + method java.lang.String io.trino.spi.type.SqlTime::toString() + @com.fasterxml.jackson.annotation.JsonValue + On-the-wire representation shouldn't rely on the Jackson format + + + true + java.annotation.removed + method java.lang.String io.trino.spi.type.SqlTimeWithTimeZone::toString() + method java.lang.String io.trino.spi.type.SqlTimeWithTimeZone::toString() + @com.fasterxml.jackson.annotation.JsonValue + On-the-wire representation shouldn't rely on the Jackson format + + + true + java.annotation.removed + method java.lang.String io.trino.spi.type.SqlTimestamp::toString() + method java.lang.String io.trino.spi.type.SqlTimestamp::toString() + @com.fasterxml.jackson.annotation.JsonValue + On-the-wire representation shouldn't rely on the Jackson format + + + true + java.annotation.removed + method java.lang.String io.trino.spi.type.SqlTimestampWithTimeZone::toString() + method java.lang.String io.trino.spi.type.SqlTimestampWithTimeZone::toString() + @com.fasterxml.jackson.annotation.JsonValue + On-the-wire representation shouldn't rely on the Jackson format + diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/SqlDate.java b/core/trino-spi/src/main/java/io/trino/spi/type/SqlDate.java index d8f092448de3..2ca4cecab692 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/SqlDate.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/SqlDate.java @@ -13,8 +13,6 @@ */ package io.trino.spi.type; -import com.fasterxml.jackson.annotation.JsonValue; - import java.time.LocalDate; public final class SqlDate @@ -51,7 +49,6 @@ public boolean equals(Object obj) return days == other.days; } - @JsonValue @Override public String toString() { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/SqlDecimal.java b/core/trino-spi/src/main/java/io/trino/spi/type/SqlDecimal.java index f84341736ad6..d25369fe6782 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/SqlDecimal.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/SqlDecimal.java @@ -13,8 +13,6 @@ */ package io.trino.spi.type; -import com.fasterxml.jackson.annotation.JsonValue; - import java.math.BigDecimal; import java.math.BigInteger; import java.math.MathContext; @@ -81,7 +79,6 @@ public int hashCode() return Objects.hash(unscaledValue, precision, scale); } - @JsonValue @Override public String toString() { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTime.java b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTime.java index a70a50a1875b..44113f652cb0 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTime.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTime.java @@ -13,8 +13,6 @@ */ package io.trino.spi.type; -import com.fasterxml.jackson.annotation.JsonValue; - import java.util.Objects; import static io.trino.spi.type.TimeType.MAX_PRECISION; @@ -85,7 +83,6 @@ public int hashCode() return Objects.hash(precision, picos); } - @JsonValue @Override public String toString() { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimeWithTimeZone.java b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimeWithTimeZone.java index dc039a09bf4a..c37c18a32b6c 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimeWithTimeZone.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimeWithTimeZone.java @@ -13,8 +13,6 @@ */ package io.trino.spi.type; -import com.fasterxml.jackson.annotation.JsonValue; - import java.util.Objects; import static io.trino.spi.type.TimeType.MAX_PRECISION; @@ -99,7 +97,6 @@ public int hashCode() return Objects.hash(precision, picos, offsetMinutes); } - @JsonValue @Override public String toString() { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestamp.java b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestamp.java index bab2fadfda55..1a2b2558adff 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestamp.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestamp.java @@ -13,8 +13,6 @@ */ package io.trino.spi.type; -import com.fasterxml.jackson.annotation.JsonValue; - import java.time.LocalDateTime; import java.time.ZoneOffset; import java.util.Objects; @@ -149,7 +147,6 @@ public int hashCode() return Objects.hash(epochMicros, picosOfMicros, precision); } - @JsonValue @Override public String toString() { diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestampWithTimeZone.java b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestampWithTimeZone.java index 7c1b6155f5ce..9eaf8133e80c 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestampWithTimeZone.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/SqlTimestampWithTimeZone.java @@ -13,8 +13,6 @@ */ package io.trino.spi.type; -import com.fasterxml.jackson.annotation.JsonValue; - import java.time.Instant; import java.time.ZoneId; import java.time.ZonedDateTime; @@ -132,7 +130,6 @@ public SqlTimestampWithTimeZone roundTo(int precision) return newInstanceWithRounding(precision, epochMillis, picosOfMilli, timeZoneKey); } - @JsonValue @Override public String toString() { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java index 46150e5975df..ec110a2407e7 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java @@ -22,10 +22,19 @@ import io.airlift.slice.SliceOutput; import io.airlift.slice.Slices; import io.trino.spi.TrinoException; +import io.trino.spi.type.SqlDate; +import io.trino.spi.type.SqlDecimal; +import io.trino.spi.type.SqlTime; +import io.trino.spi.type.SqlTimeWithTimeZone; +import io.trino.spi.type.SqlTimestamp; +import io.trino.spi.type.SqlTimestampWithTimeZone; +import io.trino.spi.type.SqlVarbinary; import java.io.IOException; import java.io.InputStreamReader; import java.io.OutputStream; +import java.util.List; +import java.util.StringJoiner; import static com.fasterxml.jackson.core.JsonFactory.Feature.CANONICALIZE_FIELD_NAMES; import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS; @@ -63,10 +72,28 @@ public static Slice jsonParse(Slice slice) } } - public static Slice toJsonValue(Object value) + public static Slice toJsonValue(List values) throws IOException { - return Slices.wrappedBuffer(SORTED_MAPPER.writeValueAsBytes(value)); + if (values == null) { + return Slices.utf8Slice("[]"); + } + + StringJoiner joiner = new StringJoiner(",", "[", "]"); + for (Object value : values) { + joiner.add(switch (value) { + case null -> "null"; + case SqlDate _, + SqlTime _, + SqlVarbinary _, + SqlTimeWithTimeZone _, + SqlDecimal _, + SqlTimestamp _, + SqlTimestampWithTimeZone _ -> SORTED_MAPPER.writeValueAsString(value.toString()); + default -> SORTED_MAPPER.writeValueAsString(value); + }); + } + return Slices.utf8Slice(joiner.toString()); } private static JsonParser createJsonParser(JsonFactory factory, Slice json) diff --git a/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java b/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java index 7cc262b79ea9..68bd81cb23ba 100644 --- a/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java +++ b/plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java @@ -1509,11 +1509,15 @@ private static SliceReadFunction arrayAsJsonReadFunction(ConnectorSession sessio type.writeObject(builder, block); Object value = type.getObjectValue(session, builder.build(), 0); + if (!(value instanceof List list)) { + throw new TrinoException(JDBC_ERROR, "Unexpected JSON object value for " + type.getDisplayName() + " expected List, got " + value.getClass().getSimpleName()); + } + try { - return toJsonValue(value); + return toJsonValue(list); } catch (IOException e) { - throw new TrinoException(JDBC_ERROR, "Conversion to JSON failed for " + type.getDisplayName(), e); + throw new TrinoException(JDBC_ERROR, "Conversion to JSON failed for " + type.getDisplayName(), e); } }; }