From 8ccce9857a472ffa2f8679e972cf6c09314d4d0f Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 20 Dec 2019 13:15:48 -0800 Subject: [PATCH] fix vectorized query engine numeric filter matchers against null values (#9063) * fix druid-sql issue with filtering numeric columns by null values * fix vector numeric column matchers to check null vector for null matches --- .../vector/DoubleVectorValueMatcher.java | 6 ++++ .../vector/FloatVectorValueMatcher.java | 6 ++++ .../filter/vector/LongVectorValueMatcher.java | 6 ++++ .../vector/VectorValueMatcherFactory.java | 34 +++++++++++++++++++ .../druid/sql/calcite/CalciteQueryTest.java | 6 ---- 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/filter/vector/DoubleVectorValueMatcher.java b/processing/src/main/java/org/apache/druid/query/filter/vector/DoubleVectorValueMatcher.java index c304a117551..8aef16e4fc7 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/vector/DoubleVectorValueMatcher.java +++ b/processing/src/main/java/org/apache/druid/query/filter/vector/DoubleVectorValueMatcher.java @@ -19,6 +19,7 @@ package org.apache.druid.query.filter.vector; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.query.filter.DruidDoublePredicate; import org.apache.druid.query.filter.DruidPredicateFactory; import org.apache.druid.segment.DimensionHandlerUtils; @@ -29,6 +30,7 @@ import javax.annotation.Nullable; public class DoubleVectorValueMatcher implements VectorValueMatcherFactory { private final VectorValueSelector selector; + private final boolean canHaveNulls = !NullHandling.replaceWithDefault(); public DoubleVectorValueMatcher(final VectorValueSelector selector) { @@ -38,6 +40,10 @@ public class DoubleVectorValueMatcher implements VectorValueMatcherFactory @Override public VectorValueMatcher makeMatcher(@Nullable final String value) { + if (value == null && canHaveNulls) { + return makeNullValueMatcher(selector); + } + final Double matchVal = DimensionHandlerUtils.convertObjectToDouble(value); if (matchVal == null) { diff --git a/processing/src/main/java/org/apache/druid/query/filter/vector/FloatVectorValueMatcher.java b/processing/src/main/java/org/apache/druid/query/filter/vector/FloatVectorValueMatcher.java index 4ea33aecb5f..f4f5a236655 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/vector/FloatVectorValueMatcher.java +++ b/processing/src/main/java/org/apache/druid/query/filter/vector/FloatVectorValueMatcher.java @@ -19,6 +19,7 @@ package org.apache.druid.query.filter.vector; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.query.filter.DruidFloatPredicate; import org.apache.druid.query.filter.DruidPredicateFactory; import org.apache.druid.segment.DimensionHandlerUtils; @@ -29,6 +30,7 @@ import javax.annotation.Nullable; public class FloatVectorValueMatcher implements VectorValueMatcherFactory { private final VectorValueSelector selector; + private final boolean canHaveNulls = !NullHandling.replaceWithDefault(); public FloatVectorValueMatcher(final VectorValueSelector selector) { @@ -38,6 +40,10 @@ public class FloatVectorValueMatcher implements VectorValueMatcherFactory @Override public VectorValueMatcher makeMatcher(@Nullable final String value) { + if (value == null && canHaveNulls) { + return makeNullValueMatcher(selector); + } + final Float matchVal = DimensionHandlerUtils.convertObjectToFloat(value); if (matchVal == null) { diff --git a/processing/src/main/java/org/apache/druid/query/filter/vector/LongVectorValueMatcher.java b/processing/src/main/java/org/apache/druid/query/filter/vector/LongVectorValueMatcher.java index a07f9ba32f0..661703c00ea 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/vector/LongVectorValueMatcher.java +++ b/processing/src/main/java/org/apache/druid/query/filter/vector/LongVectorValueMatcher.java @@ -19,6 +19,7 @@ package org.apache.druid.query.filter.vector; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.query.filter.DruidLongPredicate; import org.apache.druid.query.filter.DruidPredicateFactory; import org.apache.druid.segment.DimensionHandlerUtils; @@ -29,6 +30,7 @@ import javax.annotation.Nullable; public class LongVectorValueMatcher implements VectorValueMatcherFactory { private final VectorValueSelector selector; + private final boolean canHaveNulls = !NullHandling.replaceWithDefault(); public LongVectorValueMatcher(final VectorValueSelector selector) { @@ -38,6 +40,10 @@ public class LongVectorValueMatcher implements VectorValueMatcherFactory @Override public VectorValueMatcher makeMatcher(@Nullable final String value) { + if (value == null && canHaveNulls) { + return makeNullValueMatcher(selector); + } + final Long matchVal = DimensionHandlerUtils.convertObjectToLong(value); if (matchVal == null) { diff --git a/processing/src/main/java/org/apache/druid/query/filter/vector/VectorValueMatcherFactory.java b/processing/src/main/java/org/apache/druid/query/filter/vector/VectorValueMatcherFactory.java index a7971eb7430..e61373cc80f 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/vector/VectorValueMatcherFactory.java +++ b/processing/src/main/java/org/apache/druid/query/filter/vector/VectorValueMatcherFactory.java @@ -20,6 +20,7 @@ package org.apache.druid.query.filter.vector; import org.apache.druid.query.filter.DruidPredicateFactory; +import org.apache.druid.segment.vector.VectorValueSelector; import javax.annotation.Nullable; @@ -28,4 +29,37 @@ public interface VectorValueMatcherFactory VectorValueMatcher makeMatcher(@Nullable String value); VectorValueMatcher makeMatcher(DruidPredicateFactory predicateFactory); + + default VectorValueMatcher makeNullValueMatcher(VectorValueSelector selector) + { + return new BaseVectorValueMatcher(selector) + { + final VectorMatch match = VectorMatch.wrap(new int[selector.getMaxVectorSize()]); + + @Override + public ReadableVectorMatch match(final ReadableVectorMatch mask) + { + final boolean[] nullVector = selector.getNullVector(); + + if (nullVector == null) { + return VectorMatch.allFalse(); + } + + final int[] selection = match.getSelection(); + + int numRows = 0; + + for (int i = 0; i < mask.getSelectionSize(); i++) { + final int rowNum = mask.getSelection()[i]; + if (nullVector[rowNum]) { + selection[numRows++] = rowNum; + } + } + + match.setSelectionSize(numRows); + assert match.isValid(mask); + return match; + } + }; + } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 88b3d588f71..4116d9519d9 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -2277,8 +2277,6 @@ public class CalciteQueryTest extends BaseCalciteQueryTest @Test public void testNullLongFilter() throws Exception { - // bug https://github.com/apache/incubator-druid/issues/9062 prevents this test from passing - skipVectorize(); testQuery( "SELECT COUNT(*)\n" + "FROM druid.numfoo\n" @@ -2302,8 +2300,6 @@ public class CalciteQueryTest extends BaseCalciteQueryTest @Test public void testNullDoubleFilter() throws Exception { - // bug https://github.com/apache/incubator-druid/issues/9062 prevents this test from passing - skipVectorize(); testQuery( "SELECT COUNT(*)\n" + "FROM druid.numfoo\n" @@ -2328,8 +2324,6 @@ public class CalciteQueryTest extends BaseCalciteQueryTest @Test public void testNullFloatFilter() throws Exception { - // bug https://github.com/apache/incubator-druid/issues/9062 prevents this test from passing - skipVectorize(); testQuery( "SELECT COUNT(*)\n" + "FROM druid.numfoo\n"