diff --git a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java index a80560e332d..fb9099cbb83 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java @@ -142,12 +142,12 @@ public class InDimFilter extends AbstractOptimizableDimFilter implements Filter // The values set can be huge. Try to avoid copying the set if possible. // Note that we may still need to copy values to a list for caching. See getCacheKey(). - if (NullHandling.sqlCompatible() || !values.remove("")) { - this.values = values; - } else { + if (!NullHandling.sqlCompatible() && values.remove("")) { + // In Non sql compatible mode, empty strings should be converted to nulls for the filter. + // In sql compatible mode, empty strings and nulls should be treated differently values.add(null); - this.values = values; } + this.values = values; this.dimension = Preconditions.checkNotNull(dimension, "dimension cannot be null"); this.extractionFn = extractionFn; diff --git a/processing/src/main/java/org/apache/druid/segment/join/lookup/LookupJoinable.java b/processing/src/main/java/org/apache/druid/segment/join/lookup/LookupJoinable.java index 109da85ab46..1ecfdf1cf02 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/lookup/LookupJoinable.java +++ b/processing/src/main/java/org/apache/druid/segment/join/lookup/LookupJoinable.java @@ -20,7 +20,7 @@ package org.apache.druid.segment.join.lookup; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import org.apache.druid.java.util.common.io.Closer; import org.apache.druid.query.lookup.LookupExtractor; import org.apache.druid.segment.ColumnSelectorFactory; @@ -33,7 +33,6 @@ import org.apache.druid.segment.join.Joinable; import javax.annotation.Nullable; import java.io.Closeable; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.Set; @@ -107,10 +106,10 @@ public class LookupJoinable implements Joinable Set correlatedValues; if (LookupColumnSelectorFactory.KEY_COLUMN.equals(searchColumnName)) { if (LookupColumnSelectorFactory.KEY_COLUMN.equals(retrievalColumnName)) { - correlatedValues = ImmutableSet.of(searchColumnValue); + correlatedValues = Sets.newHashSet(searchColumnValue); } else { // This should not happen in practice because the column to be joined on must be a key. - correlatedValues = Collections.singleton(extractor.apply(searchColumnValue)); + correlatedValues = Sets.newHashSet(extractor.apply(searchColumnValue)); } } else { if (!allowNonKeyColumnSearch) { @@ -118,11 +117,11 @@ public class LookupJoinable implements Joinable } if (LookupColumnSelectorFactory.VALUE_COLUMN.equals(retrievalColumnName)) { // This should not happen in practice because the column to be joined on must be a key. - correlatedValues = ImmutableSet.of(searchColumnValue); + correlatedValues = Sets.newHashSet(searchColumnValue); } else { // Lookup extractor unapply only provides a list of strings, so we can't respect // maxCorrelationSetSize easily. This should be handled eventually. - correlatedValues = ImmutableSet.copyOf(extractor.unapply(searchColumnValue)); + correlatedValues = Sets.newHashSet(extractor.unapply(searchColumnValue)); } } return Optional.of(correlatedValues); diff --git a/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java b/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java index 9cf5b8f67bd..f54a96bff0b 100644 --- a/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java +++ b/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java @@ -21,7 +21,6 @@ package org.apache.druid.query.filter; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import org.apache.druid.common.config.NullHandling; import org.apache.druid.jackson.DefaultObjectMapper; @@ -59,26 +58,37 @@ public class InDimFilterTest extends InitializedNullHandlingTest } @Test - public void testGetValuesWithValuesSetOfNonEmptyStringsUseTheGivenSet() + public void testInitWithoutEmptyStringsShouldUseValuesAsIs() { - final Set values = ImmutableSet.of("v1", "v2", "v3"); - final InDimFilter filter = new InDimFilter("dim", values, null, null); - Assert.assertSame(values, filter.getValues()); + Set values = Sets.newHashSet("good", "bad", null); + final InDimFilter inDimFilter = new InDimFilter("dimTest", values, null); + Assert.assertSame(values, inDimFilter.getValues()); + Assert.assertEquals(3, values.size()); + Assert.assertEquals(Sets.newHashSet("good", "bad", null), inDimFilter.getValues()); } @Test - public void testGetValuesWithValuesSetIncludingEmptyString() + public void testInitWithEmptyStringsShouldUseValuesAndReplaceWithNullInNonSqlCompatibleMode() { - final Set values = Sets.newHashSet("v1", "", "v3"); - final InDimFilter filter = new InDimFilter("dim", values, null, null); - if (NullHandling.replaceWithDefault()) { - Assert.assertNotSame(values, filter.getValues()); - Assert.assertEquals(Sets.newHashSet("v1", null, "v3"), filter.getValues()); + Set values = Sets.newHashSet("good", "bad", ""); + final InDimFilter inDimFilter = new InDimFilter("dimTest", values, null); + Assert.assertSame(values, inDimFilter.getValues()); + Assert.assertEquals(3, values.size()); + if (NullHandling.sqlCompatible()) { + Assert.assertEquals(Sets.newHashSet("good", "bad", ""), inDimFilter.getValues()); } else { - Assert.assertSame(values, filter.getValues()); + Assert.assertEquals(Sets.newHashSet("good", "bad", null), inDimFilter.getValues()); } } + @Test + public void testGetValuesWithValuesSetOfNonEmptyStringsUseTheGivenSet() + { + final Set values = Sets.newHashSet("v1", "v2", "v3"); + final InDimFilter filter = new InDimFilter("dim", values, null, null); + Assert.assertSame(values, filter.getValues()); + } + @Test public void testGetCacheKeyReturningSameKeyForValuesOfDifferentOrders() {