toClassWithRealEquals take nonnull input

pengpeng-lu · pengpeng-lu · commit 35b081ccde92 · 2025-07-28T15:06:41.000-07:00
diff --git a/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/expressions/Comparisons.java b/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/expressions/Comparisons.java
@@ -57,6 +57,7 @@
 import com.apple.foundationdb.record.query.plan.cascades.BooleanWithConstraint;
 import com.apple.foundationdb.record.query.plan.cascades.Correlated;
 import com.apple.foundationdb.record.query.plan.cascades.CorrelationIdentifier;
+import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
 import com.apple.foundationdb.record.query.plan.explain.DefaultExplainFormatter;
 import com.apple.foundationdb.record.query.plan.explain.ExplainTokens;
 import com.apple.foundationdb.record.query.plan.explain.ExplainTokensWithPrecedence;
@@ -75,6 +76,7 @@
 import com.apple.foundationdb.record.util.ProtoUtils;
 import com.apple.foundationdb.tuple.ByteArrayUtil;
 import com.apple.foundationdb.tuple.ByteArrayUtil2;
+import com.apple.foundationdb.tuple.Tuple;
 import com.google.auto.service.AutoService;
 import com.google.common.base.Suppliers;
 import com.google.common.base.Verify;
@@ -213,11 +215,9 @@ private static Comparable toComparable(@Nullable Object obj) {
         }
     }
 
-    @Nullable
-    public static Object toClassWithRealEquals(@Nullable Object obj) {
-        if (obj == null) {
-            return null;
-        } else if (obj instanceof ByteString) {
+    @Nonnull
+    public static Object toClassWithRealEquals(@Nonnull Object obj) {
+        if (obj instanceof ByteString) {
             return obj;
         } else if (obj instanceof byte[]) {
             return ByteString.copyFrom((byte[])obj);
@@ -247,7 +247,7 @@ private static boolean compareEquals(@Nonnull Object value, @Nonnull Object comp
     }
 
     @SpotBugsSuppressWarnings("NP_BOOLEAN_RETURN_NULL")
-    private static boolean compareNotDistinctFrom(Object value, Object comparand) {
+    private static boolean compareNotDistinctFrom(@Nullable Object value, @Nullable Object comparand) {
         if (value == null && comparand == null) {
             return true;
         } else if (value == null || comparand == null) {
@@ -256,7 +256,7 @@ private static boolean compareNotDistinctFrom(Object value, Object comparand) {
             if (value instanceof Message) {
                 return MessageHelpers.compareMessageEquals(value, comparand);
             } else {
-                return toClassWithRealEquals(value).equals(toClassWithRealEquals(comparand));
+                return toClassWithRealEquals(Objects.requireNonNull(value)).equals(toClassWithRealEquals(Objects.requireNonNull(comparand)));
             }
         }
     }
@@ -316,7 +316,12 @@ private static Boolean compareListStartsWith(@Nullable Object value, @Nonnull Li
                 if (i > list.size()) {
                     return false;
                 }
-                if (!toClassWithRealEquals(comparand.get(i)).equals(toClassWithRealEquals(list.get(i)))) {
+                if (comparand.get(i) == null && list.get(i) == null) {
+                    continue;
+                }
+                if (comparand.get(i) == null || list.get(i) == null) {
+                    return false;
+                } else if (!toClassWithRealEquals(comparand.get(i)).equals(toClassWithRealEquals(list.get(i)))) {
                     return false;
                 }
             }
@@ -341,11 +346,12 @@ private static Boolean compareIn(@Nullable Object value, @Nullable Object compar
                         return true;
                     }
                 } else {
-                    if (toClassWithRealEquals(value).equals(toClassWithRealEquals(comparandItem))) {
+                    if (comparandItem == null) {
+                        hasNull = true;
+                    } else if (toClassWithRealEquals(value).equals(toClassWithRealEquals(comparandItem))) {
                         return true;
                     }
                 }
-                hasNull |= comparandItem == null;
             }
             return hasNull ? null : false;
         } else {
@@ -689,7 +695,7 @@ private static BiMap<Type, PComparisonType> getProtoEnumBiMap() {
     }
 
     @Nullable
-    public static Type invertComparisonType(@Nonnull final Comparisons.Type type) {
+    public static Type invertComparisonType(@Nonnull final Type type) {
         if (type.isUnary()) {
             return null;
         }
@@ -846,7 +852,7 @@ default Object getComparand() {
 
         /**
          * Get whether the comparison is with the result of a multi-column key.
-         * If so, {@link #getComparand} will return a {@link com.apple.foundationdb.tuple.Tuple}.
+         * If so, {@link #getComparand} will return a {@link Tuple}.
          * @return {@code true} if the comparand is for multiple key columns
          */
         default boolean hasMultiColumnComparand() {
@@ -1780,7 +1786,7 @@ public static class ListComparison implements Comparison {
         @SuppressWarnings("rawtypes")
         private final List comparand;
         @Nullable
-        private final Descriptors.FieldDescriptor.JavaType javaType;
+        private final JavaType javaType;
 
         @Nonnull
         @SuppressWarnings("rawtypes")
@@ -1798,8 +1804,8 @@ public ListComparison(@Nonnull Type type, @Nonnull List comparand) {
                 default:
                     throw new RecordCoreException("ListComparison only supports EQUALS, NOT_EQUALS, STARTS_WITH and IN");
             }
-            if (comparand == null || (this.type == Type.IN && comparand.stream().anyMatch(o -> o == null))) {
-                throw new NullPointerException("List comparand is null, or contains null");
+            if (this.type == Type.IN && comparand.stream().anyMatch(o -> o == null)) {
+                throw new NullPointerException("List comparand contains null");
             }
             if (comparand.isEmpty()) {
                 javaType = null;
@@ -1813,10 +1819,10 @@ public ListComparison(@Nonnull Type type, @Nonnull List comparand) {
                 }
             }
             this.comparand = comparand;
-            this.comparandListWithEqualsSupplier = Suppliers.memoize(() -> Lists.transform(comparand, Comparisons::toClassWithRealEquals));
+            this.comparandListWithEqualsSupplier = Suppliers.memoize(() -> Lists.transform(comparand, obj -> obj != null ? toClassWithRealEquals(obj) : null));
         }
 
-        private static Descriptors.FieldDescriptor.JavaType getJavaType(@Nonnull Object o) {
+        private static JavaType getJavaType(@Nonnull Object o) {
             if (o instanceof Boolean) {
                 return JavaType.BOOLEAN;
             } else if (o instanceof ByteString || o instanceof byte[]) {
diff --git a/yaml-tests/src/test/resources/aggregate-index-tests.yamsql b/yaml-tests/src/test/resources/aggregate-index-tests.yamsql
@@ -156,20 +156,8 @@ test_block:
        # At some point, should be able to roll up values from the aggregate index. However, even
        # controlling for that, it can still use the index
        - query: select max(col2) from T1 use index (mv8);
-       - supported_version: 4.1.9.0
        - explain: "ISCAN(MV8 <,>) | MAP (_ AS _0) | AGG (max_l(_._0.COL2) AS _0) | ON EMPTY NULL | MAP (_._0._0 AS _0)"
        - result: [{!l 13}]
-    -
-       - query: select max(col2) from T1 use index (mv8);
-       - maxRows: 1
-       # Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
-       # (Extra values being produced after exhausting source of an aggregate cursor)
-       # Can remove once we do not care about backwards compatibility before 4.1.9.0
-       - initialVersionLessThan: 4.1.9.0
-       # Different (incorrect) behavior for different past versions
-       - initialVersionAtLeast: 4.1.9.0
-       - result: [{!l 13}]
-       - result: []
     -
        # Min/max indexes need keep what amounts to a standard value index on their keys (in order to properly look up
        # the min/max). That index should be usable for normal queries just like a value index. Note that the scan is
@@ -183,43 +171,14 @@ test_block:
        - result: [{!l 5}, {!l 4}, {!l 3}, {!l 2}, {!l 1}]
     -
       - query: select min(col3) from T2 group by col1, col2;
-      - supported_version: 4.1.9.0
       - explain: "ISCAN(MV2 <,>) | MAP (_ AS _0) | AGG (min_l(_._0.COL3) AS _0) GROUP BY (_._0.COL1 AS _0, _._0.COL2 AS _1) | MAP (_._1._0 AS _0)"
       - result: [{!l 1}, {!l 2}, {!l 3}]
-    -
-      - query: select min(col3) from T2 group by col1, col2;
-       # Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
-       # (Extra values being produced after exhausting source of an aggregate cursor)
-       # Can remove once we do not care about backwards compatibility before 4.1.9.0
-      - maxRows: 1
-      - initialVersionLessThan: 4.1.9.0
-       # Different (incorrect) behavior for different past versions
-      - initialVersionAtLeast: 4.1.9.0
-      - result: [{!l 1}]
-      - result: [{!l 2}]
-      - result: [{!l 3}]
-      - result: []
     -
       # this should use the aggregate index in the future, for now, it is using streaming aggregate
       # over base table scan.
       - query: select max(col2) from t2;
-      - supported_version: 4.1.9.0
       - explain: "ISCAN(MV3 <,>) | MAP (_ AS _0) | AGG (max_l(_._0.COL2) AS _0) | ON EMPTY NULL | MAP (_._0._0 AS _0)"
       - result: [{!l 2}]
-    -
-      - query: select max(col2) from t2;
-       # Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
-       # (Extra values being produced after exhausting source of an aggregate cursor)
-       # Can remove once we do not care about backwards compatibility before 4.1.9.0
-      - supported_version: 4.1.9.0
-      - maxRows: 1
-      - initialVersionLessThan: 4.1.9.0
-      - result: [{!l 2}]
-      - result: [{!null _}]
-      - result: [{!l 2}] # ad infinitum
-      - initialVersionAtLeast: 4.1.9.0
-      - result: [{!l 2}]
-      - result: []
     -
       - query: select col1, sum(col2) from T1 USE INDEX (vi1) group by col1;
       - explain: "ISCAN(VI1 <,>) | MAP (_ AS _0) | AGG (sum_l(_._0.COL2) AS _0) GROUP BY (_._0.COL1 AS _0) | MAP (_._0._0 AS COL1, _._1._0 AS _1)"
@@ -273,28 +232,12 @@ test_block:
     -
       # Permuted max index can also be used to evaluate other aggregate functions via aggregation and roll-up
       - query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 asc;
-      - supported_version: 4.1.9.0
       - explain: "ISCAN(MV9 [EQUALS promote(@c20 AS LONG)]) | MAP (_ AS _0) | AGG (sum_l(_._0.COL2) AS _0) GROUP BY (_._0.COL1 AS _0, _._0.COL3 AS _1) | MAP (_._0._1 AS COL3, _._1._0 AS S)"
       - result: [{COL3: 1, S: 1}, {COL3: 2, S: 2}, {COL3: 100, S: 1}, {COL3: 200, S: 2}]
-    -
-      - query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 asc;
-      # Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
-      # (Extra values being produced after exhausting source of an aggregate cursor)
-      # Can remove once we do not care about backwards compatibility before 4.1.9.0
-      - maxRows: 0
-      - result: [{COL3: 1, S: 1}, {COL3: 2, S: 2}, {COL3: 100, S: 1}, {COL3: 200, S: 2}]
     -
       - query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 desc;
-      - supported_version: 4.1.9.0
       - explain: "ISCAN(MV9 [EQUALS promote(@c20 AS LONG)] REVERSE) | MAP (_ AS _0) | AGG (sum_l(_._0.COL2) AS _0) GROUP BY (_._0.COL1 AS _0, _._0.COL3 AS _1) | MAP (_._0._1 AS COL3, _._1._0 AS S)"
       - result: [{COL3: 200, S: 2}, {COL3: 100, S: 1}, {COL3: 2, S: 2}, {COL3: 1, S: 1}]
-    -
-      - query: select col3, sum(col2) as s from t2 use index (mv9) where col1 = 1 group by col1, col3 order by col3 desc;
-      # Cannot use FORCE_CONTINUATIONS with older versions due to: https://github.com/FoundationDB/fdb-record-layer/issues/3096
-      # (Extra values being produced after exhausting source of an aggregate cursor)
-      # Can remove once we do not care about backwards compatibility before 4.1.9.0
-      - maxRows: 0
-      - result: [{COL3: 200, S: 2}, {COL3: 100, S: 1}, {COL3: 2, S: 2}, {COL3: 1, S: 1}]
 #    -
 #      # grouping by constant is not yet supported.
 #      - query: select sum(col2) from t1 group by 3,2,1;
@@ -379,9 +322,9 @@ test_block:
     -
       - query: select b, c, d, e, max(x) from t5 where a = 0 group by a, b, c, d, e order by b, max(x), c, d, e
       - explain: "AISCAN(MV16 [EQUALS promote(@c19 AS LONG)] BY_GROUP -> [_0: KEY:[0], _1: KEY:[1], _2: KEY:[3], _3: KEY:[4], _4: KEY:[5], _5: KEY:[2]]) | MAP (_._1 AS B, _._2 AS C, _._3 AS D, _._4 AS E, _._5 AS _4)"
-      - initialVersionLessThan: !current_version
+      - initialVersionLessThan: 4.3.5.0
       - error: "0AF00"
-      - initialVersionAtLeast: !current_version
+      - initialVersionAtLeast: 4.3.5.0
       - result: [
                    {b: "bar", c: 0, d: 1, e: "e1", _4: 11},
                    {b: "bar", c: 0, d: 1, e: "e2", _4: 11},
@@ -408,9 +351,9 @@ test_block:
     -
       - query: select b, c, d, e, min(x) from t5 where a = 0 group by a, b, c, d, e order by b, min(x), c, d, e
       - explain: "AISCAN(MV17 [EQUALS promote(@c19 AS LONG)] BY_GROUP -> [_0: KEY:[0], _1: KEY:[1], _2: KEY:[3], _3: KEY:[4], _4: KEY:[5], _5: KEY:[2]]) | MAP (_._1 AS B, _._2 AS C, _._3 AS D, _._4 AS E, _._5 AS _4)"
-      - initialVersionLessThan: !current_version
+      - initialVersionLessThan: 4.3.5.0
       - error: "0AF00"
-      - initialVersionAtLeast: !current_version
+      - initialVersionAtLeast: 4.3.5.0
       - result: [
                    {b: "bar", c: 0, d: 1, e: "e1", _4:  0},
                    {b: "bar", c: 0, d: 1, e: "e2", _4:  0},
@@ -437,9 +380,9 @@ test_block:
     -
       - query: select b, c, d, e, max(x) from t5 where a = 0 group by a, b, c, d, e order by b, c, max(x), d, e
       - explain: "AISCAN(MV18 [EQUALS promote(@c19 AS LONG)] BY_GROUP -> [_0: KEY:[0], _1: KEY:[1], _2: KEY:[2], _3: KEY:[4], _4: KEY:[5], _5: KEY:[3]]) | MAP (_._1 AS B, _._2 AS C, _._3 AS D, _._4 AS E, _._5 AS _4)"
-      - initialVersionLessThan: !current_version
+      - initialVersionLessThan: 4.3.5.0
       - error: "0AF00"
-      - initialVersionAtLeast: !current_version
+      - initialVersionAtLeast: 4.3.5.0
       - result: [
                    {b: "bar", c: 0, d: 1, e: "e1", _4: 11},
                    {b: "bar", c: 0, d: 1, e: "e2", _4: 11},
@@ -466,9 +409,9 @@ test_block:
     -
       - query: select b, c, d, e, min(x) from t5 where a = 0 group by a, b, c, d, e order by b, c, min(x), d, e
       - explain: "AISCAN(MV19 [EQUALS promote(@c19 AS LONG)] BY_GROUP -> [_0: KEY:[0], _1: KEY:[1], _2: KEY:[2], _3: KEY:[4], _4: KEY:[5], _5: KEY:[3]]) | MAP (_._1 AS B, _._2 AS C, _._3 AS D, _._4 AS E, _._5 AS _4)"
-      - initialVersionLessThan: !current_version
+      - initialVersionLessThan: 4.3.5.0
       - error: "0AF00"
-      - initialVersionAtLeast: !current_version
+      - initialVersionAtLeast: 4.3.5.0
       - result: [
                    {b: "bar", c: 0, d: 1, e: "e1", _4:  0},
                    {b: "bar", c: 0, d: 1, e: "e2", _4:  0},
@@ -567,9 +510,9 @@ test_block:
     -
       - query: select b, c, d, e, max(x) from t5 where a = 0 group by a, b, c, d, e having b IN ('foo', 'bar') and d IN (1, 2) order by max(x), c, e
       - explain: "[IN @c34] INUNION q0 -> { [IN promote(@c42 AS ARRAY(LONG))] INUNION q1 -> { AISCAN(MV16 [EQUALS promote(@c19 AS LONG), EQUALS q0] BY_GROUP -> [_0: KEY:[0], _1: KEY:[1], _2: KEY:[3], _3: KEY:[4], _4: KEY:[5], _5: KEY:[2]]) | FILTER _._3 EQUALS q1 | MAP (_._1 AS B, _._2 AS C, _._3 AS D, _._4 AS E, _._5 AS _4) } COMPARE BY (_._4, _.C, _.E, _.D) } COMPARE BY (_._4, _.C, _.E, _.B)"
-      - initialVersionLessThan: !current_version
+      - initialVersionLessThan: 4.3.5.0
       - error: "0AF00"
-      - initialVersionAtLeast: !current_version
+      - initialVersionAtLeast: 4.3.5.0
       - result: [
                    {b: "foo", c: 0, d: 2, e: "e2", _4:  8},
                    {b: "foo", c: 3, d: 1, e: "e1", _4:  8},
@@ -583,9 +526,9 @@ test_block:
     -
       - query: select b, c, d, e, min(x) from t5 where a = 0 group by a, b, c, d, e having b IN ('foo', 'bar') and d IN (1, 2) order by min(x), c, e
       - explain: "[IN @c34] INUNION q0 -> { [IN promote(@c42 AS ARRAY(LONG))] INUNION q1 -> { AISCAN(MV17 [EQUALS promote(@c19 AS LONG), EQUALS q0] BY_GROUP -> [_0: KEY:[0], _1: KEY:[1], _2: KEY:[3], _3: KEY:[4], _4: KEY:[5], _5: KEY:[2]]) | FILTER _._3 EQUALS q1 | MAP (_._1 AS B, _._2 AS C, _._3 AS D, _._4 AS E, _._5 AS _4) } COMPARE BY (_._4, _.C, _.E, _.D) } COMPARE BY (_._4, _.C, _.E, _.B)"
-      - initialVersionLessThan: !current_version
+      - initialVersionLessThan: 4.3.5.0
       - error: "0AF00"
-      - initialVersionAtLeast: !current_version
+      - initialVersionAtLeast: 4.3.5.0
       - result: [
                    {b: "bar", c: 0, d: 1, e: "e1", _4:  0},
                    {b: "bar", c: 0, d: 1, e: "e2", _4:  0},
