Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ public static String emptyToNullIfNeeded(@Nullable String value)
//CHECKSTYLE.ON: Regexp
}

public static boolean needsEmptyToNull(@Nullable String value)
{
return replaceWithDefault() && Strings.isNullOrEmpty(value);
}

@Nullable
public static String defaultStringValue()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ public DataSegment fetchUsedSegment(String dataSource, String segmentId)
DATA_SOURCE,
testCase.interval,
null,
new TrueDimFilter(),
TrueDimFilter.instance(),
Arrays.asList(DIMENSIONS),
Arrays.asList(METRICS),
// Split as much as possible
Expand Down
7 changes: 5 additions & 2 deletions processing/src/main/java/org/apache/druid/query/Druids.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.java.util.common.granularity.Granularity;
import org.apache.druid.query.aggregation.AggregatorFactory;
Expand Down Expand Up @@ -58,6 +58,7 @@
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
*/
Expand Down Expand Up @@ -202,7 +203,9 @@ public TimeseriesQueryBuilder filters(String dimensionName, String value)

public TimeseriesQueryBuilder filters(String dimensionName, String value, String... values)
{
dimFilter = new InDimFilter(dimensionName, Lists.asList(value, values), null, null);
final Set<String> filterValues = Sets.newHashSet(values);
filterValues.add(value);
dimFilter = new InDimFilter(dimensionName, filterValues, null, null);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

/**
*/
Expand Down Expand Up @@ -72,8 +73,20 @@ public byte[] getCacheKey()
@Override
public DimFilter optimize()
{
List<DimFilter> elements = DimFilters.optimize(fields);
return elements.size() == 1 ? elements.get(0) : new AndDimFilter(elements);
List<DimFilter> elements = DimFilters.optimize(fields)
.stream()
.filter(filter -> !(filter instanceof TrueDimFilter))
.collect(Collectors.toList());
if (elements.isEmpty()) {
// All elements were TrueDimFilter after optimization
return TrueDimFilter.instance();
} else if (elements.size() == 1) {
return elements.get(0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AndDimFilterTest is missing branch coverage for this and line 88

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this code is hit by other tests in druid-processing, but I'm not sure which ones

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of this and OrDimFilter are prevalent in unit tests so I believe they are being tested somewhere (I didn't change the logic of the lines you mentioned). But I can add some more tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

} else if (elements.stream().anyMatch(filter -> filter instanceof FalseDimFilter)) {
return FalseDimFilter.instance();
} else {
return new AndDimFilter(elements);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
@JsonSubTypes.Type(name = "interval", value = IntervalDimFilter.class),
@JsonSubTypes.Type(name = "like", value = LikeDimFilter.class),
@JsonSubTypes.Type(name = "expression", value = ExpressionDimFilter.class),
@JsonSubTypes.Type(name = "true", value = TrueDimFilter.class)
@JsonSubTypes.Type(name = "true", value = TrueDimFilter.class),
@JsonSubTypes.Type(name = "false", value = FalseDimFilter.class)
})
public interface DimFilter extends Cacheable
{
Expand Down Expand Up @@ -78,6 +79,7 @@ public interface DimFilter extends Cacheable
* @return a RangeSet that represent the possible range of the input dimension, or null if it is not possible to
* determine for this DimFilter.
*/
@Nullable
RangeSet<String> getDimensionRangeSet(String dimension);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class DimFilterUtils
static final byte COLUMN_COMPARISON_CACHE_ID = 0xD;
static final byte EXPRESSION_CACHE_ID = 0xE;
static final byte TRUE_CACHE_ID = 0xF;
static final byte FALSE_CACHE_ID = 0x11;
public static final byte BLOOM_DIM_FILTER_CACHE_ID = 0x10;

public static final byte STRING_SEPARATOR = (byte) 0xFF;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.druid.query.filter;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.google.common.collect.ImmutableRangeSet;
import com.google.common.collect.RangeSet;
import org.apache.druid.query.cache.CacheKeyBuilder;
import org.apache.druid.segment.filter.FalseFilter;

import javax.annotation.Nullable;
import java.util.Collections;
import java.util.Set;

public class FalseDimFilter implements DimFilter
{
private static final FalseDimFilter INSTANCE = new FalseDimFilter();
private static final byte[] CACHE_KEY = new CacheKeyBuilder(DimFilterUtils.FALSE_CACHE_ID).build();

@JsonCreator
public static FalseDimFilter instance()
{
return INSTANCE;
}

private FalseDimFilter()
{
}

@Override
public DimFilter optimize()
{
return this;
}

@Override
public Filter toFilter()
{
return FalseFilter.instance();
}

@Nullable
@Override
public RangeSet<String> getDimensionRangeSet(String dimension)
{
return ImmutableRangeSet.of();
}

@Override
public Set<String> getRequiredColumns()
{
return Collections.emptySet();
}

@Override
public byte[] getCacheKey()
{
return CACHE_KEY;
}

@Override
public int hashCode()
{
return DimFilterUtils.FALSE_CACHE_ID;
}

@Override
public boolean equals(Object o)
{
return o != null && o.getClass() == this.getClass();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EqualsVerifierTest for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


@Override
public String toString()
{
return "FALSE";
}
}
Loading