fix array presenting columns to not match single element arrays to sc…

…alars for equality
clintropolis · Dec 6, 2023 · fb77785 · fb77785
1 parent b66d995
commit fb77785
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 11 deletions.
diff --git a/processing/src/main/java/org/apache/druid/query/filter/EqualityFilter.java b/processing/src/main/java/org/apache/druid/query/filter/EqualityFilter.java
@@ -392,6 +392,9 @@ public DruidDoublePredicate makeDoublePredicate()
     @Override
     public Predicate<Object[]> makeArrayPredicate(@Nullable TypeSignature<ValueType> arrayType)
     {
+      if (!matchValue.isArray()) {
+        return Predicates.alwaysFalse();
+      }
       if (arrayType == null) {
         // fall back to per row detection if input array type is unknown
         return typeDetectingArrayPredicateSupplier.get();

diff --git a/processing/src/main/java/org/apache/druid/segment/nested/VariantColumnAndIndexSupplier.java b/processing/src/main/java/org/apache/druid/segment/nested/VariantColumnAndIndexSupplier.java
@@ -320,6 +320,9 @@ private class ArrayValueIndexes implements ValueIndexes
     @Override
     public BitmapColumnIndex forValue(@Nonnull Object value, TypeSignature<ValueType> valueType)
     {
+      if (!valueType.isArray()) {
+        return new AllFalseBitmapColumnIndex(bitmapFactory, nullValueBitmap);
+      }
       final ExprEval<?> eval = ExprEval.ofType(ExpressionType.fromColumnTypeStrict(valueType), value);
       final ExprEval<?> castForComparison = ExprEval.castForEqualityComparison(
           eval,

diff --git a/processing/src/test/java/org/apache/druid/segment/filter/EqualityFilterTests.java b/processing/src/test/java/org/apache/druid/segment/filter/EqualityFilterTests.java
@@ -412,22 +412,18 @@ public void testMultiValueStringColumn()
     {
       if (isAutoSchema()) {
         // auto ingests arrays instead of strings
-        // single values are implicitly upcast to single element arrays, so we get some matches here...
         if (NullHandling.sqlCompatible()) {
-          assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "", null), ImmutableList.of("2"));
+          assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "", null), ImmutableList.of());
+          assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING_ARRAY, ImmutableList.of(""), null), ImmutableList.of("2"));
         }
-        assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "a", null), ImmutableList.of("3"));
+        assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "a", null), ImmutableList.of());
+        assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING_ARRAY, ImmutableList.of("a"), null), ImmutableList.of("3"));
         assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "b", null), ImmutableList.of());
-        assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "c", null), ImmutableList.of("4"));
+        assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "c", null), ImmutableList.of());
+        assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING_ARRAY, ImmutableList.of("c"), null), ImmutableList.of("4"));
         assertFilterMatches(new EqualityFilter("dim2", ColumnType.STRING, "d", null), ImmutableList.of());
 
         // array matchers can match the whole array
-        if (NullHandling.sqlCompatible()) {
-          assertFilterMatches(
-              new EqualityFilter("dim2", ColumnType.STRING, ImmutableList.of(""), null),
-              ImmutableList.of("2")
-          );
-        }
         assertFilterMatches(
             new EqualityFilter("dim2", ColumnType.STRING_ARRAY, new Object[]{"a", "b"}, null),
             ImmutableList.of("0")
@@ -994,7 +990,7 @@ public void testVariant()
       "3", .. [1.1, 2.2, 3.3]
       "4", .. 12.34
       "5", .. [100, 200, 300]
-      
+
        */
       Assume.assumeTrue(isAutoSchema());
       assertFilterMatches(
@@ -1018,13 +1014,23 @@ public void testVariant()
           ImmutableList.of("0", "1", "2", "3", "4", "5")
       );
 
+      // variant columns must be matched as arrays if they contain any arrays
       assertFilterMatches(
           new EqualityFilter(
               "variant",
               ColumnType.STRING,
               "abc",
               null
           ),
+          ImmutableList.of()
+      );
+      assertFilterMatches(
+          new EqualityFilter(
+              "variant",
+              ColumnType.STRING_ARRAY,
+              ImmutableList.of("abc"),
+              null
+          ),
           ImmutableList.of("0")
       );
 
@@ -1035,6 +1041,15 @@ public void testVariant()
               100L,
               null
           ),
+          ImmutableList.of()
+      );
+      assertFilterMatches(
+          new EqualityFilter(
+              "variant",
+              ColumnType.LONG_ARRAY,
+              ImmutableList.of(100L),
+              null
+          ),
           ImmutableList.of("1", "2")
       );
 
@@ -1045,6 +1060,15 @@ public void testVariant()
               "100",
               null
           ),
+          ImmutableList.of()
+      );
+      assertFilterMatches(
+          new EqualityFilter(
+              "variant",
+              ColumnType.STRING_ARRAY,
+              new Object[]{"100"},
+              null
+          ),
           ImmutableList.of("1", "2")
       );
 
@@ -1255,6 +1279,7 @@ public void test_equals()
                         "cachedOptimizedFilter"
                     )
                     .withPrefabValues(ColumnType.class, ColumnType.STRING, ColumnType.DOUBLE)
+                    .withPrefabValues(ExprEval.class, ExprEval.of("hello"), ExprEval.of(1.0))
                     .withIgnoredFields("predicateFactory", "cachedOptimizedFilter", "matchValue")
                     .verify();
     }