GoncaloCoutoDosSantos opened a new pull request, #5199:
URL: https://github.com/apache/calcite/pull/5199

     ## Jira Link                                                               
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     [CALCITE-7716](https://issues.apache.org/jira/browse/CALCITE-7716)         
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     ## Changes Proposed                                                        
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     SQL orders `UUID` values as unsigned 128-bit integers, but 
`java.util.UUID#compareTo`                                                      
                                                                    
     compares the two 64-bit halves as *signed* longs. Since a `UUID` was used 
directly as                                                                     
                                                     
     the `Comparable` value of a `UUID` `RexLiteral`, every range predicate 
inherited that                                                                  
                                                        
     signed ordering.                                                           
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     Any bound whose most significant bit is set therefore sorts *below* one 
whose is not, so                                                                
                                                       
     `RexSimplify` sees an inverted (empty) range and folds the predicate away: 
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     ```sql                                                                     
                                                                                
                                                    
     SELECT UUID '8ba7b810-9dad-11d1-80b4-00c04fd430c8'                         
                                                                                
                                                    
       BETWEEN UUID '00000000-0000-0000-0000-000000000000'                      
                                                                                
                                                    
           AND UUID 'ffffffff-ffff-ffff-ffff-ffffffffffff';                     
                                                                                
                                                    
     ```                                                                        
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     Returns `FALSE` before this change, `TRUE` after. `ffffffff-…` has all 
bits set, so its                                                                
                                                        
     high half is `-1` as a signed long and it compares as less than 
`00000000-…`, making the                                                        
                                                               
     range empty. The same applies to `<`, `<=`, `>` and `>=`; `IN`, `NOT IN` 
and                                                                             
                                                      
     `IS [NOT] DISTINCT FROM` were unaffected because they compare by equality 
only.                                                                           
                                                     
                    
   ### The fix                                                                  
                                                                                
                                                  
                                                                                
                                                                                
                                                    
     Introduce `org.apache.calcite.util.UuidValue`, a small wrapper around 
`java.util.UUID`                                                                
                                                         
     that implements `Comparable` using `Long.compareUnsigned` on each half, 
and use it as                                                                   
                                                       
     both the `RexLiteral` value and the runtime representation of SQL `UUID`:  
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     - `RexLiteral` — `valueMatchesType` and the `UUID` assertion now expect 
`UuidValue`.                                                                    
                                                       
       `getValueAs(UUID.class)` is still supported and unwraps, so existing 
callers keep working.                                                           
                                                        
     - `RexBuilder` — `makeUuidLiteral` wraps into a `UuidValue`; a `UuidValue` 
overload is                                                                     
                                                    
       added and `makeLiteral` accepts either representation.                   
                                                                                
                                                    
     - `JavaTypeFactoryImpl` — SQL `UUID` now maps to `UuidValue.class` rather 
than `UUID.class`.                                                              
                                                     
     - `SqlFunctions` — `uuidToString`, `uuidToBinary` and `binaryToUuid` 
operate on                                                                      
                                                          
       `UuidValue`, plus new `lt`/`le`/`gt`/`ge` overloads so runtime 
comparison uses the same                                                        
                                                              
       unsigned ordering as planning-time simplification.                       
                                                                                
                                                    
     - `BuiltInMethod`, `RelJson`, `VariantNonNull` — updated to the new type.  
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     `UuidValue.fromString` (wired to `BuiltInMethod.UUID_FROM_STRING`, used by 
the runtime                                                                     
                                                    
     `CAST(VARCHAR AS UUID)` path) delegates to the existing lenient 
`SqlFunctions.stringToUuid`                                                     
                                                               
     rather 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 old `UUID#compareTo` ordering for anyone 
depending on the                                                                
                                                       
     previous behaviour.                                                        
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     ### Tests                                                                  
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     `SqlOperatorTest.testUuidBetween` covers `BETWEEN`/`NOT BETWEEN`, `<` and 
`>` across the                                                                  
                                                     
     minimum, a mid-range and the maximum UUID, and asserts that `IN`, `NOT IN` 
and                                                                             
                                                    
     `IS [NOT] DISTINCT FROM` are independent of the ordering. Expected values 
are derived from                                                                
                                                     
     `UUID_UNSIGNED_COMPARISON` so the test is correct under either setting.    
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     `./gradlew build` passes, including the full test suite. 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to