diff --git a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java index 39ba5c2f3281..f04a16fa05a7 100644 --- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java +++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java @@ -114,6 +114,13 @@ public final class CalciteSystemProperty { public static final CalciteSystemProperty 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 + * [CALCITE-7716]. */ + public static final CalciteSystemProperty UUID_UNSIGNED_COMPARISON = + booleanProperty("calcite.uuid.unsigned.comparison", true); + /** * Whether to include a GraphViz representation when dumping the state of the * Volcano planner. diff --git a/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java b/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java index 2114921971d4..bcdeada9c37f 100644 --- a/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java +++ b/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: diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java index 8fa5abd149d8..b8b0a7f86b76 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java +++ b/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: " diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java index 667de88227f5..26a146c96305 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java +++ b/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 + "'"); diff --git a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java index 545e81995d0d..a51480691297 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java +++ b/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: diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java index d50d7279a538..480a73363bca 100644 --- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java +++ b/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 < operator applied to UUID values. */ + public static boolean lt(UuidValue b0, UuidValue b1) { + return b0.compareTo(b1) < 0; + } + /** SQL < 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 ≤ operator applied to UUID values. */ + public static boolean le(UuidValue b0, UuidValue b1) { + return b0.compareTo(b1) <= 0; + } + /** SQL ≤ 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 > operator applied to UUID values. */ + public static boolean gt(UuidValue b0, UuidValue b1) { + return b0.compareTo(b1) > 0; + } + /** SQL > 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 ≥ operator applied to UUID values. */ + public static boolean ge(UuidValue b0, UuidValue b1) { + return b0.compareTo(b1) >= 0; + } + /** SQL ≥ operator applied to BigDecimal values. */ public static boolean ge(BigDecimal b0, BigDecimal b1) { return b0.compareTo(b1) >= 0; diff --git a/core/src/main/java/org/apache/calcite/runtime/variant/VariantNonNull.java b/core/src/main/java/org/apache/calcite/runtime/variant/VariantNonNull.java index 9c288e3058cc..f21211519fb8 100644 --- a/core/src/main/java/org/apache/calcite/runtime/variant/VariantNonNull.java +++ b/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: diff --git a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java index 948b62f28816..731d420f3448 100644 --- a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java +++ b/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), diff --git a/core/src/main/java/org/apache/calcite/util/UuidValue.java b/core/src/main/java/org/apache/calcite/util/UuidValue.java new file mode 100644 index 000000000000..33795d012a49 --- /dev/null +++ b/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. + * + *

Exists because {@link UUID#compareTo} compares the two 64-bit halves as + * signed longs, whereas SQL orders UUIDs as unsigned 128-bit values; see + * [CALCITE-7716]. + */ +public class UuidValue implements Comparable { + 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(); + } +} diff --git a/site/_docs/history.md b/site/_docs/history.md index d1e0caae3a71..c3696ebebb75 100644 --- a/site/_docs/history.md +++ b/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. +* [CALCITE-7716] +`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. + +* [CALCITE-7716] +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. + * [CALCITE-7727] 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. diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java index 6082c27638df..c0b88d6f2202 100644 --- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java +++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java @@ -19,6 +19,7 @@ import org.apache.calcite.avatica.util.ByteString; import org.apache.calcite.avatica.util.DateTimeUtils; import org.apache.calcite.config.CalciteConnectionProperty; +import org.apache.calcite.config.CalciteSystemProperty; import org.apache.calcite.linq4j.Linq4j; import org.apache.calcite.linq4j.function.Function1; import org.apache.calcite.linq4j.function.Function2; @@ -515,6 +516,39 @@ protected SqlOperatorFixture fixture() { false); } + /** Test case for + * [CALCITE-7716] + * BETWEEN/range predicates on UUID literals give wrong results because + * RexSimplify orders bounds using java.util.UUID#compareTo (signed + * comparison). Results depend on + * {@link CalciteSystemProperty#UUID_UNSIGNED_COMPARISON}. */ + @Test void testUuidBetween() { + final SqlOperatorFixture f = fixture(); + final String mid = "UUID '8ba7b810-9dad-11d1-80b4-00c04fd430c8'"; + final String min = "UUID '00000000-0000-0000-0000-000000000000'"; + final String max = "UUID 'ffffffff-ffff-ffff-ffff-ffffffffffff'"; + final boolean unsigned = + CalciteSystemProperty.UUID_UNSIGNED_COMPARISON.value(); + + // Under signed ordering 'ffffffff-...' compares as less than + // '00000000-...', so the range is empty and BETWEEN folds to FALSE. + f.checkBoolean(mid + " between " + min + " and " + max, unsigned); + f.checkBoolean(min + " between " + min + " and " + max, unsigned); + f.checkBoolean(max + " between " + min + " and " + max, unsigned); + f.checkBoolean(mid + " not between " + min + " and " + max, !unsigned); + f.checkBoolean(mid + " > " + min, unsigned); + f.checkBoolean(max + " < " + min, !unsigned); + + // IN, NOT IN and IS DISTINCT FROM also build Sargs, but compare by + // equality, so their results do not depend on the ordering. + f.checkBoolean(mid + " in (" + min + ", " + max + ")", false); + f.checkBoolean(max + " in (" + min + ", " + max + ")", true); + f.checkBoolean(mid + " not in (" + min + ", " + max + ")", true); + f.checkBoolean(max + " not in (" + min + ", " + max + ")", false); + f.checkBoolean(max + " is distinct from " + min, true); + f.checkBoolean(max + " is not distinct from " + max, true); + } + /** Test case for * Sql validator limits decimal literals to 64 bits. */ @Test void testLargeLiterals() {