Cleans up datum comparator

partiql · Dec 13, 2024 · 9f97db1 · 9f97db1
1 parent 3afdc4e
commit 9f97db1
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 29 deletions.
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallDynamic.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallDynamic.kt
@@ -181,10 +181,6 @@ internal class ExprCallDynamic(
                 if (function.isNullCall && arg.isNull) {
                     return nil.invoke()
                 }
-                // TODO
-//                if (arg.isMissing && function.isMissingCall) {
-//                    return Datum.missing(function.returns)
-//                }
                 val argType = arg.type
                 val paramType = function.parameters[i]
                 when (paramType == argType) {

diff --git a/partiql-spi/src/main/java/org/partiql/spi/value/DatumComparator.java b/partiql-spi/src/main/java/org/partiql/spi/value/DatumComparator.java
@@ -100,7 +100,36 @@ abstract class DatumComparator implements Comparator<Datum> {
 
     @Override
     public int compare(Datum lhs, Datum rhs) {
-        // Check for NULL/MISSING
+        // Check if  NULL/MISSING
+        Integer result = checkUnknown(lhs, rhs);
+        if (result != null) {
+            return result;
+        }
+
+        // Check for VARIANT & if NULL/MISSING
+        boolean lhsIsVariant = lhs.getType().code() == PType.VARIANT;
+        boolean rhsIsVariant = rhs.getType().code() == PType.VARIANT;
+        Datum lhsActual = lhsIsVariant ? lhs.lower() : lhs;
+        Datum rhsActual = rhsIsVariant ? rhs.lower() : rhs;
+        if (lhsIsVariant || rhsIsVariant) {
+            result = checkUnknown(lhsActual, rhsActual);
+            if (result != null) {
+                return result;
+            }
+        }
+
+        // Invoke the Comparison Table
+        int lhsKind = lhsActual.getType().code();
+        int rhsKind = rhsActual.getType().code();
+        return COMPARISON_TABLE[lhsKind][rhsKind].apply(lhsActual, rhsActual, this);
+    }
+
+    /**
+     * @param lhs the left side
+     * @param rhs the right side
+     * @return null if both are NOT unknown (AKA, they are both concrete); the result if one or more is unknown.
+     */
+    private Integer checkUnknown(Datum lhs, Datum rhs) {
         boolean lhsIsUnknown = lhs.isNull() || lhs.isMissing();
         boolean rhsIsUnknown = rhs.isNull() || rhs.isMissing();
         if (lhsIsUnknown && rhsIsUnknown) {
@@ -112,30 +141,7 @@ public int compare(Datum lhs, Datum rhs) {
         if (rhsIsUnknown) {
             return rhsUnknown();
         }
-
-        // Lower (if needed)
-        Datum lhsActual = lhs.getType().code() == PType.VARIANT ? lhs.lower() : lhs;
-        Datum rhsActual = rhs.getType().code() == PType.VARIANT ? rhs.lower() : rhs;
-
-
-        // TODO: Consolidate
-
-        boolean lhsActualIsUnknown = lhsActual.isNull() || lhsActual.isMissing();
-        boolean rhsActualIsUnknown = rhsActual.isNull() || rhsActual.isMissing();
-        if (lhsActualIsUnknown && rhsActualIsUnknown) {
-            return EQUAL;
-        }
-        if (lhsActualIsUnknown) {
-            return lhsUnknown();
-        }
-        if (rhsActualIsUnknown) {
-            return rhsUnknown();
-        }
-
-        // Invoke the Comparison Table
-        int lhsKind = lhsActual.getType().code();
-        int rhsKind = rhsActual.getType().code();
-        return COMPARISON_TABLE[lhsKind][rhsKind].apply(lhsActual, rhsActual, this);
+        return null;
     }
 
     /**