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.
trinodb · Nov 22, 2024 · e52c451 · e52c451
1 parent 4ecf64a
commit e52c451
Show file tree

Hide file tree

Showing 10 changed files with 96 additions and 21 deletions.
diff --git a/...in/src/test/java/io/trino/operator/aggregation/TestQuantileDigestAggregationFunction.java b/...in/src/test/java/io/trino/operator/aggregation/TestQuantileDigestAggregationFunction.java
@@ -34,6 +34,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.stream.Collectors;
 import java.util.stream.LongStream;
 
@@ -413,4 +414,12 @@ private Number getUpperBound(double error, List<? extends Number> rows, double p
         int marginOfError = (int) (rows.size() * error / 2);
         return rows.get(min(medianIndex + marginOfError, rows.size() - 1));
     }
+
+
+    public static void main(String[] args) {
+        String value = "ΝΕΣΤΟΡΑΣ ΒΛΣΧΟΣ I.K.E.";
+        System.out.println(value);
+        System.out.println(value.toLowerCase(Locale.ENGLISH));
+        System.out.println(value.toLowerCase(Locale.ENGLISH).equals("νεστορας βλσχος i.k.e."));
+    }
 }
diff --git a/core/trino-spi/pom.xml b/core/trino-spi/pom.xml
@@ -225,6 +225,54 @@
                                     <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
                                     <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
                                 </item>
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.annotation.removed</code>
+                                    <old>method java.lang.String io.trino.spi.type.SqlDate::toString()</old>
+                                    <new>method java.lang.String io.trino.spi.type.SqlDate::toString()</new>
+                                    <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
+                                    <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
+                                </item>
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.annotation.removed</code>
+                                    <old>method java.lang.String io.trino.spi.type.SqlDecimal::toString()</old>
+                                    <new>method java.lang.String io.trino.spi.type.SqlDecimal::toString()</new>
+                                    <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
+                                    <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
+                                </item>
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.annotation.removed</code>
+                                    <old>method java.lang.String io.trino.spi.type.SqlTime::toString()</old>
+                                    <new>method java.lang.String io.trino.spi.type.SqlTime::toString()</new>
+                                    <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
+                                    <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
+                                </item>
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.annotation.removed</code>
+                                    <old>method java.lang.String io.trino.spi.type.SqlTimeWithTimeZone::toString()</old>
+                                    <new>method java.lang.String io.trino.spi.type.SqlTimeWithTimeZone::toString()</new>
+                                    <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
+                                    <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
+                                </item>
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.annotation.removed</code>
+                                    <old>method java.lang.String io.trino.spi.type.SqlTimestamp::toString()</old>
+                                    <new>method java.lang.String io.trino.spi.type.SqlTimestamp::toString()</new>
+                                    <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
+                                    <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
+                                </item>
+                                <item>
+                                    <ignore>true</ignore>
+                                    <code>java.annotation.removed</code>
+                                    <old>method java.lang.String io.trino.spi.type.SqlTimestampWithTimeZone::toString()</old>
+                                    <new>method java.lang.String io.trino.spi.type.SqlTimestampWithTimeZone::toString()</new>
+                                    <annotation>@com.fasterxml.jackson.annotation.JsonValue</annotation>
+                                    <justification>On-the-wire representation shouldn't rely on the Jackson format</justification>
+                                </item>
                             </differences>
                         </revapi.differences>
                     </analysisConfiguration>

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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -16,16 +16,26 @@
 import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import io.airlift.json.ObjectMapperProvider;
 import io.airlift.slice.DynamicSliceOutput;
 import io.airlift.slice.Slice;
 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 static com.fasterxml.jackson.core.JsonFactory.Feature.CANONICALIZE_FIELD_NAMES;
 import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS;
@@ -63,10 +73,32 @@ 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("[]");
+        }
+
+        ImmutableList.Builder<String> json = ImmutableList.builder();
+        for (Object value : values) {
+            if (value == null) {
+                json.add("null");
+                continue;
+            }
+            json.add(switch (value) {
+                case SqlDate _,
+                     SqlTime _,
+                     SqlVarbinary _,
+                     SqlTimeWithTimeZone _,
+                     SqlDecimal _,
+                     SqlTimestamp _,
+                     SqlTimestampWithTimeZone _ -> SORTED_MAPPER.writeValueAsString(value.toString());
+                default -> SORTED_MAPPER.writeValueAsString(value);
+            });
+        }
+
+        return Slices.utf8Slice("[" + Joiner.on(",").join(json.build()) + "]");
     }
 
     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
@@ -1474,8 +1474,12 @@ 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, "Conversion to JSON failed for  " + type.getDisplayName());
+            }
+
             try {
-                return toJsonValue(value);
+                return toJsonValue(list);
             }
             catch (IOException e) {
                 throw new TrinoException(JDBC_ERROR, "Conversion to JSON failed for  " + type.getDisplayName(), e);