[CALCITE-7716] BETWEEN/range predicates on UUID literals give wrong results because RexSimplify orders bounds using java.util.UUID#compareTo (signed comparison)

core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java

@@ -114,6 +114,13 @@ public final class CalciteSystemProperty<T> {
   public static final CalciteSystemProperty<Boolean> STRICT =
       booleanProperty("calcite.strict.sql", false);
 
+  /** Whether to order UUIDs as unsigned 128-bit values, as SQL requires,
+   * rather than using {@link java.util.UUID#compareTo}, which treats each half
+   * as signed; see
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-7716">[CALCITE-7716]</a>. */
+  public static final CalciteSystemProperty<Boolean> UUID_UNSIGNED_COMPARISON =
+      booleanProperty("calcite.uuid.unsigned.comparison", true);
+
   /**
    * Whether to include a GraphViz representation when dumping the state of the
    * Volcano planner.

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java

@@ -36,6 +36,7 @@
 import org.apache.calcite.sql.type.SqlTypeUtil;
 import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.Util;
+import org.apache.calcite.util.UuidValue;
 
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.joou.UByte;
@@ -53,7 +54,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.UUID;
 import java.util.stream.Collectors;
 
 import static org.apache.calcite.util.ReflectUtil.isStatic;
@@ -225,10 +225,10 @@ private static Type fieldType(Field field) {
       case BINARY:
       case VARBINARY:
         return ByteString.class;
+      case UUID:
+        return UuidValue.class;
       case GEOMETRY:
         return Geometry.class;
-      case UUID:
-        return UUID.class;
       case SYMBOL:
         return Enum.class;
       case ANY:

core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java

@@ -78,6 +78,7 @@
 import org.apache.calcite.util.TimeString;
 import org.apache.calcite.util.TimestampString;
 import org.apache.calcite.util.Util;
+import org.apache.calcite.util.UuidValue;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationFeature;
@@ -101,7 +102,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.UUID;
 
 import static org.apache.calcite.rel.RelDistributions.EMPTY;
 import static org.apache.calcite.util.Static.RESOURCE;
@@ -510,7 +510,7 @@ public Object toJson(AggregateCall node) {
       return toJson((Range) value);
     } else if (value instanceof ByteString) {
       return toJson(((ByteString) value).toString(16));
-    } else if (value instanceof UUID) {
+    } else if (value instanceof UuidValue) {
       return toJson(value.toString());
     } else {
       throw new UnsupportedOperationException("type not serializable as JSON: "

core/src/main/java/org/apache/calcite/rex/RexBuilder.java

@@ -60,6 +60,7 @@
 import org.apache.calcite.util.TimestampString;
 import org.apache.calcite.util.TimestampWithTimeZoneString;
 import org.apache.calcite.util.Util;
+import org.apache.calcite.util.UuidValue;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableRangeSet;
@@ -1572,7 +1573,12 @@ public RexLiteral makeLiteral(boolean b) {
   }
 
   public RexLiteral makeUuidLiteral(@Nullable UUID uuid) {
-    return new RexLiteral(uuid, typeFactory.createSqlType(SqlTypeName.UUID), SqlTypeName.UUID);
+    UuidValue uuidValue = (uuid != null) ? new UuidValue(uuid) : null;
+    return new RexLiteral(uuidValue, typeFactory.createSqlType(SqlTypeName.UUID), SqlTypeName.UUID);
+  }
+
+  public RexLiteral makeUuidLiteral(@Nullable UuidValue uuidValue) {
+    return new RexLiteral(uuidValue, typeFactory.createSqlType(SqlTypeName.UUID), SqlTypeName.UUID);
   }
 
   /**
@@ -2370,7 +2376,11 @@ public RexNode makeLiteral(@Nullable Object value, RelDataType type,
     case ANY:
       return makeLiteral(value, guessType(value), allowCast);
     case UUID:
-      return makeUuidLiteral((UUID) value);
+      if (value instanceof UUID) {
+        return makeUuidLiteral((UUID) value);
+      } else {
+        return makeUuidLiteral((UuidValue) value);
+      }
     default:
       throw new IllegalArgumentException(
           "Cannot create literal for type '" + sqlTypeName + "'");

core/src/main/java/org/apache/calcite/rex/RexLiteral.java

@@ -44,6 +44,7 @@
 import org.apache.calcite.util.TimestampString;
 import org.apache.calcite.util.TimestampWithTimeZoneString;
 import org.apache.calcite.util.Util;
+import org.apache.calcite.util.UuidValue;
 
 import com.google.common.collect.ImmutableList;
 
@@ -323,7 +324,7 @@ public static boolean valueMatchesType(
     }
     switch (typeName) {
     case UUID:
-      return value instanceof UUID;
+      return value instanceof UuidValue;
     case VARIANT:
       return value instanceof VariantValue;
     case BOOLEAN:
@@ -707,7 +708,7 @@ private static void appendAsJava(@Nullable Comparable value, StringBuilder sb,
           printSarg(sb2, (Sarg) value, type));
       break;
     case UUID:
-      assert value instanceof UUID;
+      assert value instanceof UuidValue;
       sb.append(value);
       break;
     case SYMBOL:
@@ -1079,7 +1080,9 @@ public boolean isNull() {
     switch (typeName) {
     case UUID:
       if (clazz == String.class) {
-        return clazz.cast(((UUID) value).toString());
+        return clazz.cast(value.toString());
+      } else if (clazz == UUID.class) {
+        return clazz.cast(((UuidValue) value).uuid());
       }
       break;
     case BINARY:

core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java

@@ -50,6 +50,7 @@
 import org.apache.calcite.util.TryThreadLocal;
 import org.apache.calcite.util.Unsafe;
 import org.apache.calcite.util.Util;
+import org.apache.calcite.util.UuidValue;
 import org.apache.calcite.util.format.FormatElement;
 import org.apache.calcite.util.format.FormatModel;
 import org.apache.calcite.util.format.FormatModels;
@@ -348,7 +349,7 @@ public static boolean throwUnless(boolean condition, String message) {
     return condition;
   }
 
-  public static String uuidToString(UUID uuid) {
+  public static String uuidToString(UuidValue uuid) {
     return uuid.toString();
   }
 
@@ -413,17 +414,17 @@ public static UUID stringToUuid(String s) {
         Long.parseUnsignedLong(digits.substring(16), 16));
   }
 
-  public static UUID binaryToUuid(ByteString bytes) {
+  public static UuidValue binaryToUuid(ByteString bytes) {
     if (bytes.length() != 16) {
       throw new IllegalArgumentException("Need exactly 16 bytes for UUID");
     }
     ByteBuffer byteBuffer = ByteBuffer.wrap(bytes.getBytes());
     long mostSignificantBits = byteBuffer.getLong();
     long leastSignificantBits = byteBuffer.getLong();
-    return new UUID(mostSignificantBits, leastSignificantBits);
+    return new UuidValue(new UUID(mostSignificantBits, leastSignificantBits));
   }
 
-  public static ByteString uuidToBinary(UUID uuid) {
+  public static ByteString uuidToBinary(UuidValue uuid) {
     byte[] dest = new byte[16];
     ByteBuffer byteBuffer = ByteBuffer.wrap(dest);
     byteBuffer.putLong(uuid.getMostSignificantBits());
@@ -2553,6 +2554,11 @@ public static boolean lt(ByteString b0, ByteString b1) {
     return b0.compareTo(b1) < 0;
   }
 
+  /** SQL <code>&lt;</code> operator applied to UUID values. */
+  public static boolean lt(UuidValue b0, UuidValue b1) {
+    return b0.compareTo(b1) < 0;
+  }
+
   /** SQL <code>&lt;</code> operator applied to BigDecimal values. */
   public static boolean lt(BigDecimal b0, BigDecimal b1) {
     return b0.compareTo(b1) < 0;
@@ -2631,6 +2637,11 @@ public static boolean le(ByteString b0, ByteString b1) {
     return b0.compareTo(b1) <= 0;
   }
 
+  /** SQL <code>&le;</code> operator applied to UUID values. */
+  public static boolean le(UuidValue b0, UuidValue b1) {
+    return b0.compareTo(b1) <= 0;
+  }
+
   /** SQL <code>&le;</code> operator applied to BigDecimal values. */
   public static boolean le(BigDecimal b0, BigDecimal b1) {
     return b0.compareTo(b1) <= 0;
@@ -2679,6 +2690,11 @@ public static boolean gt(ByteString b0, ByteString b1) {
     return b0.compareTo(b1) > 0;
   }
 
+  /** SQL <code>&gt;</code> operator applied to UUID values. */
+  public static boolean gt(UuidValue b0, UuidValue b1) {
+    return b0.compareTo(b1) > 0;
+  }
+
   /** SQL <code>&gt;</code> operator applied to BigDecimal values. */
   public static boolean gt(BigDecimal b0, BigDecimal b1) {
     return b0.compareTo(b1) > 0;
@@ -2758,6 +2774,11 @@ public static boolean ge(ByteString b0, ByteString b1) {
     return b0.compareTo(b1) >= 0;
   }
 
+  /** SQL <code>&ge;</code> operator applied to UUID values. */
+  public static boolean ge(UuidValue b0, UuidValue b1) {
+    return b0.compareTo(b1) >= 0;
+  }
+
   /** SQL <code>&ge;</code> operator applied to BigDecimal values. */
   public static boolean ge(BigDecimal b0, BigDecimal b1) {
     return b0.compareTo(b1) >= 0;

core/src/main/java/org/apache/calcite/runtime/variant/VariantNonNull.java

@@ -22,6 +22,7 @@
 import org.apache.calcite.runtime.rtti.BasicSqlTypeRtti;
 import org.apache.calcite.runtime.rtti.RowSqlTypeRtti;
 import org.apache.calcite.runtime.rtti.RuntimeTypeInformation;
+import org.apache.calcite.util.UuidValue;
 
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.joou.UByte;
@@ -37,7 +38,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.UUID;
 
 import static org.apache.calcite.runtime.rtti.RuntimeTypeInformation.RuntimeSqlTypeName.NAME;
 
@@ -55,7 +55,7 @@ public class VariantNonNull extends VariantSqlValue {
     // sanity check
     switch (runtimeType.getTypeName()) {
     case UUID:
-      assert value instanceof UUID;
+      assert value instanceof UuidValue;
       this.value = value;
       break;
     case NAME:

core/src/main/java/org/apache/calcite/util/BuiltInMethod.java

@@ -150,7 +150,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.TimeZone;
-import java.util.UUID;
 import java.util.function.BiPredicate;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -546,9 +545,9 @@ public enum BuiltInMethod {
   IS_JSON_ARRAY(JsonFunctions.class, "isJsonArray", String.class),
   IS_JSON_SCALAR(JsonFunctions.class, "isJsonScalar", String.class),
   ST_GEOM_FROM_EWKT(SpatialTypeFunctions.class, "ST_GeomFromEWKT", String.class),
-  UUID_FROM_STRING(SqlFunctions.class, "stringToUuid", String.class),
-  UUID_TO_STRING(SqlFunctions.class, "uuidToString", UUID.class),
-  UUID_TO_BINARY(SqlFunctions.class, "uuidToBinary", UUID.class),
+  UUID_FROM_STRING(UuidValue.class, "fromString", String.class),
+  UUID_TO_STRING(SqlFunctions.class, "uuidToString", UuidValue.class),
+  UUID_TO_BINARY(SqlFunctions.class, "uuidToBinary", UuidValue.class),
   INT_TO_BINARY(SqlFunctions.class, "intToBinary", Object.class, int.class, boolean.class),
   BINARY_TO_UUID(SqlFunctions.class, "binaryToUuid", ByteString.class),
   INITCAP(SqlFunctions.class, "initcap", String.class),

core/src/main/java/org/apache/calcite/util/UuidValue.java

@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import org.apache.calcite.config.CalciteSystemProperty;
+import org.apache.calcite.runtime.SqlFunctions;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.UUID;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A UUID value; the value of a UUID {@link org.apache.calcite.rex.RexLiteral}
+ * and the runtime representation of a UUID.
+ *
+ * <p>Exists because {@link UUID#compareTo} compares the two 64-bit halves as
+ * signed longs, whereas SQL orders UUIDs as unsigned 128-bit values; see
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7716">[CALCITE-7716]</a>.
+ */
+public class UuidValue implements Comparable<UuidValue> {
+  private static final boolean UNSIGNED_COMPARISON =
+      CalciteSystemProperty.UUID_UNSIGNED_COMPARISON.value();
+
+  private final UUID uuid;
+
+  public UuidValue(UUID uuid) {
+    this.uuid = requireNonNull(uuid, "uuid");
+  }
+
+  /** Creates a UuidValue from any of the spellings accepted by
+   * {@link SqlFunctions#stringToUuid}, in which hyphens are optional group
+   * separators; called from generated code. */
+  public static UuidValue fromString(String s) {
+    return new UuidValue(SqlFunctions.stringToUuid(s));
+  }
+
+  /** Returns the wrapped {@link UUID}. */
+  public UUID uuid() {
+    return uuid;
+  }
+
+  /** Returns the least significant 64 bits of this UUID. */
+  public long getLeastSignificantBits() {
+    return uuid.getLeastSignificantBits();
+  }
+
+  /** Returns the most significant 64 bits of this UUID. */
+  public long getMostSignificantBits() {
+    return uuid.getMostSignificantBits();
+  }
+
+  /** Compares two UUIDs as unsigned 128-bit values, or, if
+   * {@link CalciteSystemProperty#UUID_UNSIGNED_COMPARISON} is off, using
+   * {@link UUID#compareTo}. */
+  @Override public int compareTo(UuidValue that) {
+    if (!UNSIGNED_COMPARISON) {
+      return uuid.compareTo(that.uuid);
+    }
+    final int c =
+        Long.compareUnsigned(uuid.getMostSignificantBits(),
+            that.uuid.getMostSignificantBits());
+    if (c != 0) {
+      return c;
+    }
+    return Long.compareUnsigned(uuid.getLeastSignificantBits(),
+        that.uuid.getLeastSignificantBits());
+  }
+
+  @Override public boolean equals(@Nullable Object obj) {
+    return this == obj
+        || obj instanceof UuidValue
+        && uuid.equals(((UuidValue) obj).uuid);
+  }
+
+  @Override public int hashCode() {
+    return uuid.hashCode();
+  }
+
+  @Override public String toString() {
+    return uuid.toString();
+  }
+}

site/_docs/history.md

@@ -64,6 +64,23 @@ Class loading from model files has been disabled by default. Any attempt to load
 classes from model files will lead to `SecurityException` unless an appropriate
 pattern is set in `calcite.model.classes.allowed` system property.
 
+* [<a href="https://issues.apache.org/jira/browse/CALCITE-7716">CALCITE-7716</a>]
+`UUID` values are now ordered as unsigned 128-bit values, as SQL requires.
+Previously ordering used `java.util.UUID.compareTo`, which compares each 64-bit
+half as a signed value, so `ffffffff-ffff-ffff-ffff-ffffffffffff` sorted below
+`00000000-0000-0000-0000-000000000000`. `ORDER BY`, `BETWEEN`, `<`, `<=`, `>`,
+`>=`, `MIN` and `MAX` on a `UUID` may therefore give different results. Set
+`calcite.uuid.unsigned.comparison` to `false` to restore the previous ordering.
+
+* [<a href="https://issues.apache.org/jira/browse/CALCITE-7716">CALCITE-7716</a>]
+The runtime and `RexLiteral` representation of a `UUID` is now
+`org.apache.calcite.util.UuidValue` rather than `java.util.UUID`.
+`JavaTypeFactoryImpl` maps `SqlTypeName.UUID` to `UuidValue`, so generated code
+and adapters see a `UuidValue`, and `RexLiteral.getValue` returns one, although
+`getValueAs(UUID.class)` still returns a `java.util.UUID`. The `SqlFunctions`
+methods `uuidToString` and `uuidToBinary` now take a `UuidValue`, and
+`binaryToUuid` returns one.
+
 * [<a href="https://issues.apache.org/jira/browse/CALCITE-7727">CALCITE-7727</a>]
 Comparing a `UUID` with a character or binary value now converts that value to a
 `UUID`, the same direction as comparing a string with a number or a datetime.