Skip to content

Remap id's returned in XXXFilteredDimensionSpec.getRow() as per reduced cardinality #2267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fjy merged 2 commits into from
Jan 15, 2016
Merged

Remap id's returned in XXXFilteredDimensionSpec.getRow() as per reduced cardinality #2267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3140b2
fix topN filtering on multi-valued dimension bug
binlijin Jan 12, 2016
ae6a111
fix XXXFilteredDimensionSpec to remap the dictionary encodings as per…
himanshug Jan 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions processing/src/main/java/io/druid/query/dimension/BaseFilteredDimensionSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Preconditions;
import io.druid.query.extraction.ExtractionFn;
import io.druid.segment.DimensionSelector;
import io.druid.segment.data.IndexedInts;
import io.druid.segment.data.ListBasedIndexedInts;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

/**
*/
Expand Down Expand Up @@ -65,4 +72,51 @@ public boolean preservesOrdering()
{
return delegate.preservesOrdering();
}

protected static DimensionSelector decorate(
final DimensionSelector selector,
final Map<Integer, Integer> forwardMapping,
final int[] reverseMapping
)
{
if (selector == null) {
return selector;
}

return new DimensionSelector()
{
@Override
public IndexedInts getRow()
{
IndexedInts baseRow = selector.getRow();
List<Integer> result = new ArrayList<>(baseRow.size());

for (int i : baseRow) {
if (forwardMapping.containsKey(i)) {
result.add(forwardMapping.get(i));
}
}

return new ListBasedIndexedInts(result);
}

@Override
public int getValueCardinality()
{
return forwardMapping.size();
}

@Override
public String lookupName(int id)
{
return selector.lookupName(reverseMapping[id]);
}

@Override
public int lookupId(String name)
{
return forwardMapping.get(selector.lookupId(name));
}
};
}
}
78 changes: 26 additions & 52 deletions processing/src/main/java/io/druid/query/dimension/ListFilteredDimensionSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.metamx.common.StringUtils;
import io.druid.query.filter.DimFilterCacheHelper;
import io.druid.segment.DimensionSelector;
import io.druid.segment.data.IndexedInts;
import io.druid.segment.data.ListBasedIndexedInts;

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

/**
Expand All @@ -40,12 +38,12 @@ public class ListFilteredDimensionSpec extends BaseFilteredDimensionSpec

private static final byte CACHE_TYPE_ID = 0x3;

private final List<String> values;
private final Set<String> values;
private final boolean isWhitelist;

public ListFilteredDimensionSpec(
@JsonProperty("delegate") DimensionSpec delegate,
@JsonProperty("values") List<String> values,
@JsonProperty("values") Set<String> values,
@JsonProperty("isWhitelist") Boolean isWhitelist
)
{
Expand All @@ -58,7 +56,7 @@ public ListFilteredDimensionSpec(
}
Copy link
Copy Markdown
Contributor

@guobingkun guobingkun Jan 14, 2016

Choose a reason for hiding this comment

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

line55 could be:

this.isWhitelist = isWhitelist == null || isWhitelist.booleanValue();


@JsonProperty
public List<String> getValues()
public Set<String> getValues()
{
return values;
}
Expand All @@ -76,55 +74,31 @@ public DimensionSelector decorate(final DimensionSelector selector)
return selector;
}

final Set<Integer> matched = new HashSet<>(values.size());
for (String value : values) {
int i = selector.lookupId(value);
if (i >= 0) {
matched.add(i);
}
};

return new DimensionSelector()
{
@Override
public IndexedInts getRow()
{
IndexedInts baseRow = selector.getRow();
List<Integer> result = new ArrayList<>(baseRow.size());

for (int i : baseRow) {
if (matched.contains(i)) {
if (isWhitelist) {
result.add(i);
}
} else {
if (!isWhitelist) {
result.add(i);
}
}
}
int selectorCardinality = selector.getValueCardinality();
int cardinality = isWhitelist ? values.size() : selectorCardinality - values.size();

return new ListBasedIndexedInts(result);
}
int count = 0;
final Map<Integer,Integer> forwardMapping = new HashMap<>(cardinality);
final int[] reverseMapping = new int[cardinality];

@Override
public int getValueCardinality()
{
return matched.size();
if (isWhitelist) {
for (String value : values) {
int i = selector.lookupId(value);
if (i >= 0) {
forwardMapping.put(i, count);
reverseMapping[count++] = i;
}
}

@Override
public String lookupName(int id)
{
return selector.lookupName(id);
} else {
for (int i = 0; i < selectorCardinality; i++) {
if (!values.contains(Strings.nullToEmpty(selector.lookupName(i)))) {
forwardMapping.put(i, count);
reverseMapping[count++] = i;
}
}
}

@Override
public int lookupId(String name)
{
return selector.lookupId(name);
}
};
return BaseFilteredDimensionSpec.decorate(selector, forwardMapping, reverseMapping);
}

@Override
Expand Down
57 changes: 15 additions & 42 deletions processing/src/main/java/io/druid/query/dimension/RegexFilteredDimensionSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,10 @@
import com.metamx.common.StringUtils;
import io.druid.query.filter.DimFilterCacheHelper;
import io.druid.segment.DimensionSelector;
import io.druid.segment.data.IndexedInts;
import io.druid.segment.data.ListBasedIndexedInts;

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.BitSet;
import java.util.List;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -68,48 +65,24 @@ public DimensionSelector decorate(final DimensionSelector selector)
return selector;
}

final BitSet bitSetOfIds = new BitSet(selector.getValueCardinality());
for (int i = 0; i < selector.getValueCardinality(); i++) {
if (compiledRegex.matcher(Strings.nullToEmpty(selector.lookupName(i))).matches()) {
bitSetOfIds.set(i);
}
if (selector == null) {
return selector;
}

return new DimensionSelector()
{
@Override
public IndexedInts getRow()
{
IndexedInts baseRow = selector.getRow();
List<Integer> result = new ArrayList<>(baseRow.size());

for (int i : baseRow) {
if (bitSetOfIds.get(i)) {
result.add(i);
}
}

return new ListBasedIndexedInts(result);
}
int count = 0;
final Map<Integer,Integer> forwardMapping = new HashMap<>();

@Override
public int getValueCardinality()
{
return bitSetOfIds.cardinality();
}

@Override
public String lookupName(int id)
{
return selector.lookupName(id);
for (int i = 0; i < selector.getValueCardinality(); i++) {
if (compiledRegex.matcher(Strings.nullToEmpty(selector.lookupName(i))).matches()) {
forwardMapping.put(i, count++);
}
}

@Override
public int lookupId(String name)
{
return selector.lookupId(name);
}
};
final int[] reverseMapping = new int[forwardMapping.size()];
for (Map.Entry<Integer, Integer> e : forwardMapping.entrySet()) {
reverseMapping[e.getValue().intValue()] = e.getKey().intValue();
}
return BaseFilteredDimensionSpec.decorate(selector, forwardMapping, reverseMapping);
}

@Override
Expand Down
68 changes: 68 additions & 0 deletions processing/src/test/java/io/druid/query/MultiValuedDimensionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.io.Files;
import com.metamx.common.guava.Sequence;
import com.metamx.common.guava.Sequences;
Expand All @@ -37,11 +39,18 @@
import io.druid.query.aggregation.CountAggregatorFactory;
import io.druid.query.dimension.DefaultDimensionSpec;
import io.druid.query.dimension.DimensionSpec;
import io.druid.query.dimension.ListFilteredDimensionSpec;
import io.druid.query.dimension.RegexFilteredDimensionSpec;
import io.druid.query.filter.SelectorDimFilter;
import io.druid.query.groupby.GroupByQuery;
import io.druid.query.groupby.GroupByQueryRunnerTestHelper;
import io.druid.query.spec.LegacySegmentSpec;
import io.druid.query.topn.TopNQuery;
import io.druid.query.topn.TopNQueryBuilder;
import io.druid.query.topn.TopNQueryConfig;
import io.druid.query.topn.TopNQueryQueryToolChest;
import io.druid.query.topn.TopNQueryRunnerFactory;
import io.druid.query.topn.TopNResultValue;
import io.druid.segment.IncrementalIndexSegment;
import io.druid.segment.IndexSpec;
import io.druid.segment.QueryableIndex;
Expand All @@ -50,6 +59,7 @@
import io.druid.segment.incremental.IncrementalIndex;
import io.druid.segment.incremental.OnheapIncrementalIndex;
import org.apache.commons.io.FileUtils;
import org.joda.time.DateTime;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -58,6 +68,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

/**
*/
Expand Down Expand Up @@ -250,6 +261,63 @@ public void testGroupByWithDimFilterAndWithFilteredDimSpec() throws Exception
TestHelper.assertExpectedObjects(expectedResults, Sequences.toList(result, new ArrayList<Row>()), "");
}

@Test
public void testTopNWithDimFilterAndWithFilteredDimSpec() throws Exception
{
TopNQuery query = new TopNQueryBuilder()
.dataSource("xx")
.granularity(QueryGranularity.ALL)
.dimension(new ListFilteredDimensionSpec(
new DefaultDimensionSpec("tags", "tags"),
ImmutableSet.of("t3"),
null
))
.metric("count")
.intervals(QueryRunnerTestHelper.fullOnInterval)
.aggregators(
Arrays.asList(
new AggregatorFactory[]
{
new CountAggregatorFactory("count")
}
))
.threshold(5)
.filters(new SelectorDimFilter("tags", "t3")).build();

QueryRunnerFactory factory = new TopNQueryRunnerFactory(
TestQueryRunners.getPool(),
new TopNQueryQueryToolChest(
new TopNQueryConfig(),
QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator()
),
QueryRunnerTestHelper.NOOP_QUERYWATCHER
);
QueryRunner<Result<TopNResultValue>> runner = QueryRunnerTestHelper.makeQueryRunner(
factory,
new QueryableIndexSegment("sid1", queryableIndex)
);
Map<String, Object> context = Maps.newHashMap();
Sequence<Result<TopNResultValue>> result = runner.run(query, context);
List<Result<TopNResultValue>> expectedResults = Arrays.asList(
new Result<TopNResultValue>(
new DateTime("2011-01-12T00:00:00.000Z"),
new TopNResultValue(
Arrays.<Map<String, Object>>asList(
ImmutableMap.<String, Object>of(
"tags", "t3",
"count", 2L
)
)
)
)
);
TestHelper.assertExpectedObjects(
expectedResults,
Sequences.toList(result, new ArrayList<Result<TopNResultValue>>()),
""
);
}

@AfterClass
public static void cleanup() throws Exception
{
Expand Down
Loading