[CALCITE-7716] BETWEEN/range predicates on UUID literals give wrong results because RexSimplify orders bounds using java.util.UUID#compareTo (signed comparison) - #5199
Conversation
…esults because RexSimplify orders bounds using java.util.UUID#compareTo (signed comparison)
mihaibudiu
left a comment
There was a problem hiding this comment.
this is a breaking change, both in APIs and semantics, at the very least it has to be documented in the release notes
Is the History.md file the right place to document the changes, i did not find any file referencing the release notes. |
Correct @GoncaloCoutoDosSantos , history.md contains a commented out section for the next release (1.43 in this case). In there you can find (commented out) the breaking changes for 1.43 (so that the Release Manager don't forget about them when the release is ready). You can add a new item in there. |
b98f184 to
4040495
Compare
|



Jira Link
CALCITE-7716
Changes Proposed
SQL orders
UUIDvalues as unsigned 128-bit integers, butjava.util.UUID#compareTocompares the two 64-bit halves as signed longs. Since a
UUIDwas used directly asthe
Comparablevalue of aUUIDRexLiteral, every range predicate inherited thatsigned ordering.
Any bound whose most significant bit is set therefore sorts below one whose is not, so
RexSimplifysees an inverted (empty) range and folds the predicate away:Returns
FALSEbefore this change,TRUEafter.ffffffff-…has all bits set, so itshigh half is
-1as a signed long and it compares as less than00000000-…, making therange empty. The same applies to
<,<=,>and>=;IN,NOT INandIS [NOT] DISTINCT FROMwere unaffected because they compare by equality only.The fix
Introduce
org.apache.calcite.util.UuidValue, a small wrapper aroundjava.util.UUIDthat implements
ComparableusingLong.compareUnsignedon each half, and use it asboth the
RexLiteralvalue and the runtime representation of SQLUUID:RexLiteral—valueMatchesTypeand theUUIDassertion now expectUuidValue.getValueAs(UUID.class)is still supported and unwraps, so existing callers keep working.RexBuilder—makeUuidLiteralwraps into aUuidValue; aUuidValueoverload isadded and
makeLiteralaccepts either representation.JavaTypeFactoryImpl— SQLUUIDnow maps toUuidValue.classrather thanUUID.class.SqlFunctions—uuidToString,uuidToBinaryandbinaryToUuidoperate onUuidValue, plus newlt/le/gt/geoverloads so runtime comparison uses the sameunsigned ordering as planning-time simplification.
BuiltInMethod,RelJson,VariantNonNull— updated to the new type.UuidValue.fromString(wired toBuiltInMethod.UUID_FROM_STRING, used by the runtimeCAST(VARCHAR AS UUID)path) delegates to the existing lenientSqlFunctions.stringToUuidrather than
java.util.UUID.fromString, so all the spellings Calcite already accepts —optional hyphen group separators, surrounding braces — keep working, matching PostgreSQL.
Escape hatch
The new system property
calcite.uuid.unsigned.comparison(CalciteSystemProperty.UUID_UNSIGNED_COMPARISON,default
true) reverts to the oldUUID#compareToordering for anyone depending on theprevious behaviour.
Tests
SqlOperatorTest.testUuidBetweencoversBETWEEN/NOT BETWEEN,<and>across theminimum, a mid-range and the maximum UUID, and asserts that
IN,NOT INandIS [NOT] DISTINCT FROMare independent of the ordering. Expected values are derived fromUUID_UNSIGNED_COMPARISONso the test is correct under either setting../gradlew buildpasses, including the full test suite.