From 7ce05d58bc96aa447310bd5b75aad9b6e4616ec1 Mon Sep 17 00:00:00 2001 From: kaijianding Date: Fri, 24 Feb 2017 00:07:59 +0800 Subject: [PATCH] fix NPE in search query when dimension contains null value (#3968) * fix NPE when dimension contains null value in search query * add ut * search with not existed dimension should always return empty result --- .../druid/query/search/SearchQueryRunner.java | 6 +- .../query/search/SearchQueryRunnerTest.java | 73 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java b/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java index 4a6903e0ba5..cde2e4567db 100644 --- a/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java +++ b/processing/src/main/java/io/druid/query/search/SearchQueryRunner.java @@ -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; @@ -41,6 +42,7 @@ import io.druid.segment.ColumnValueSelector; 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; @@ -126,12 +128,12 @@ public class SearchQueryRunner implements QueryRunner> final Object2IntRBTreeMap 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; } diff --git a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java index 5ccea9eacfe..2086b339593 100644 --- a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java @@ -22,6 +22,8 @@ package io.druid.query.search; 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; @@ -29,8 +31,10 @@ 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; @@ -49,9 +53,14 @@ import io.druid.query.search.search.SearchQuery; 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; @@ -732,6 +741,70 @@ public class SearchQueryRunnerTest checkSearchQuery(searchQuery, expectedHits); } + @Test + public void testSearchWithNullValueInDimension() throws Exception + { + IncrementalIndex 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.of("name", "name1", "host", "host") + ) + ); + index.add( + new MapBasedInputRow( + 1481871670000L, + Arrays.asList("name", "table"), + ImmutableMap.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.of("searchStrategy", "cursorOnly")) + .build(); + + QueryRunnerFactory factory = new SearchQueryRunnerFactory( + selector, + toolChest, + QueryRunnerTestHelper.NOOP_QUERYWATCHER + ); + QueryRunner runner = factory.createRunner(new QueryableIndexSegment("asdf", TestIndex.persistRealtimeAndLoadMMapped(index))); + List 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 noHit = Lists.newLinkedList(); + checkSearchQuery(searchQuery, noHit); + } + private void checkSearchQuery(Query searchQuery, List expectedResults) { checkSearchQuery(searchQuery, runner, expectedResults);