Skip to content

Commit

Permalink
fix NPE in search query when dimension contains null value (apache#3968)
Browse files Browse the repository at this point in the history 
* fix NPE when dimension contains null value in search query

* add ut

* search with not existed dimension should always return empty result
  • Loading branch information
@kaijianding @gianm
kaijianding authored and gianm committed Feb 23, 2017
1 parent ebd100c commit 7ce05d5
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 2 deletions.
6 changes: 4 additions & 2 deletions processing/src/main/java/io/druid/query/search/SearchQueryRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package io.druid.query.search;

import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
Expand All @@ -41,6 +42,7 @@
import io.druid.segment.DimensionSelector;
import io.druid.segment.FloatColumnSelector;
import io.druid.segment.LongColumnSelector;
import io.druid.segment.NullDimensionSelector;
import io.druid.segment.Segment;
import io.druid.segment.column.ColumnCapabilities;
import io.druid.segment.column.ValueType;
Expand Down Expand Up @@ -126,12 +128,12 @@ public void updateSearchResultSet(
final Object2IntRBTreeMap<SearchHit> set
)
{
if (selector != null) {
if (selector != null && !(selector instanceof NullDimensionSelector)) {
final IndexedInts vals = selector.getRow();
for (int i = 0; i < vals.size(); ++i) {
final String dimVal = selector.lookupName(vals.get(i));
if (searchQuerySpec.accept(dimVal)) {
set.addTo(new SearchHit(outputName, dimVal), 1);
set.addTo(new SearchHit(outputName, Strings.nullToEmpty(dimVal)), 1);
if (set.size() >= limit) {
return;
}
Expand Down
73 changes: 73 additions & 0 deletions processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import io.druid.data.input.MapBasedInputRow;
import io.druid.granularity.QueryGranularities;
import io.druid.java.util.common.guava.Sequence;
import io.druid.java.util.common.guava.Sequences;
import io.druid.java.util.common.logger.Logger;
import io.druid.js.JavaScriptConfig;
import io.druid.query.Druids;
import io.druid.query.Query;
import io.druid.query.QueryRunner;
import io.druid.query.QueryRunnerFactory;
import io.druid.query.QueryRunnerTestHelper;
import io.druid.query.Result;
import io.druid.query.aggregation.Aggregator;
import io.druid.query.dimension.DefaultDimensionSpec;
import io.druid.query.dimension.ExtractionDimensionSpec;
import io.druid.query.extraction.ExtractionFn;
Expand All @@ -49,9 +53,14 @@
import io.druid.query.search.search.SearchQueryConfig;
import io.druid.query.search.search.SearchSortSpec;
import io.druid.query.spec.MultipleIntervalSegmentSpec;
import io.druid.segment.QueryableIndexSegment;
import io.druid.segment.TestHelper;
import io.druid.segment.TestIndex;
import io.druid.segment.column.Column;
import io.druid.segment.column.ValueType;
import io.druid.segment.incremental.IncrementalIndex;
import io.druid.segment.incremental.IncrementalIndexSchema;
import io.druid.segment.incremental.OnheapIncrementalIndex;
import org.joda.time.DateTime;
import org.joda.time.Interval;
import org.junit.Assert;
Expand Down Expand Up @@ -732,6 +741,70 @@ public void testSearchOnFloatColumnWithExFn()
checkSearchQuery(searchQuery, expectedHits);
}

@Test
public void testSearchWithNullValueInDimension() throws Exception
{
IncrementalIndex<Aggregator> index = new OnheapIncrementalIndex(
new IncrementalIndexSchema.Builder()
.withQueryGranularity(QueryGranularities.NONE)
.withMinTimestamp(new DateTime("2011-01-12T00:00:00.000Z").getMillis()).build(),
true,
10
);
index.add(
new MapBasedInputRow(
1481871600000L,
Arrays.asList("name", "host"),
ImmutableMap.<String, Object>of("name", "name1", "host", "host")
)
);
index.add(
new MapBasedInputRow(
1481871670000L,
Arrays.asList("name", "table"),
ImmutableMap.<String, Object>of("name", "name2", "table", "table")
)
);

SearchQuery searchQuery = Druids.newSearchQueryBuilder()
.dimensions(
new DefaultDimensionSpec("table", "table")
)
.dataSource(QueryRunnerTestHelper.dataSource)
.granularity(QueryRunnerTestHelper.allGran)
.intervals(QueryRunnerTestHelper.fullOnInterval)
// simulate when cardinality is big enough to fallback to cursorOnly strategy
.context(ImmutableMap.<String, Object>of("searchStrategy", "cursorOnly"))
.build();

QueryRunnerFactory factory = new SearchQueryRunnerFactory(
selector,
toolChest,
QueryRunnerTestHelper.NOOP_QUERYWATCHER
);
QueryRunner runner = factory.createRunner(new QueryableIndexSegment("asdf", TestIndex.persistRealtimeAndLoadMMapped(index)));
List<SearchHit> expectedHits = Lists.newLinkedList();
expectedHits.add(new SearchHit("table", "table", 1));
expectedHits.add(new SearchHit("table", "", 1));
checkSearchQuery(searchQuery, runner, expectedHits);
}

@Test
public void testSearchWithNotExistedDimension() throws Exception
{
SearchQuery searchQuery = Druids.newSearchQueryBuilder()
.dimensions(
new DefaultDimensionSpec("asdf", "asdf")
)
.dataSource(QueryRunnerTestHelper.dataSource)
.granularity(QueryRunnerTestHelper.allGran)
.intervals(QueryRunnerTestHelper.fullOnInterval)
.build();

List<SearchHit> noHit = Lists.newLinkedList();
checkSearchQuery(searchQuery, noHit);
}

private void checkSearchQuery(Query searchQuery, List<SearchHit> expectedResults)
{
checkSearchQuery(searchQuery, runner, expectedResults);
Expand Down

0 comments on commit 7ce05d5

Please sign in to comment.