Skip to content

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

Merged
fjy merged 2 commits intoapache:masterfrom
himanshug:fix_topn_multi_val_filter
Jan 15, 2016
Merged

Remap id's returned in XXXFilteredDimensionSpec.getRow() as per reduced cardinality#2267
fjy merged 2 commits intoapache:masterfrom
himanshug:fix_topn_multi_val_filter

Conversation

@himanshug
Copy link
Copy Markdown
Contributor

fixes the issue described in # #2255

@cheddar
Copy link
Copy Markdown
Contributor

cheddar commented Jan 14, 2016

👍

@binlijin
Copy link
Copy Markdown
Contributor

Some place use the dimension cardinality before the dimension decorate, if this has no problem i think this can be accepted.

public class TopNQueryEngine
{
  public Sequence<Result<TopNResultValue>> query(final TopNQuery query, final StorageAdapter adapter)
  {
   ...
    final Function<Cursor, Result<TopNResultValue>> mapFn = getMapFn(query, adapter);

    return Sequences.filter(
        Sequences.map(
            adapter.makeCursors(filter, queryIntervals.get(0), granularity),
            new Function<Cursor, Result<TopNResultValue>>()
            {
              @Override
              public Result<TopNResultValue> apply(Cursor input)
              {
                log.debug("Running over cursor[%s]", adapter.getInterval(), input.getTime());
                return mapFn.apply(input);
              }
            }
        ),
        Predicates.<Result<TopNResultValue>>notNull()
    );
  }

  private Function<Cursor, Result<TopNResultValue>> getMapFn(TopNQuery query, final StorageAdapter adapter)
  {
    final Capabilities capabilities = adapter.getCapabilities();
    final String dimension = query.getDimensionSpec().getDimension();

   //before dimension decorate
    final int cardinality = adapter.getDimensionCardinality(dimension); 
    ...
    final TopNAlgorithmSelector selector = new TopNAlgorithmSelector(cardinality, numBytesPerRecord);

    final TopNAlgorithm topNAlgorithm;
    ...
    return new TopNMapFn(query, topNAlgorithm);
  }
}

public class TopNMapFn implements Function<Cursor, Result<TopNResultValue>>
{
  public Result<TopNResultValue> apply(Cursor cursor)
  {
    // dimension decorate
    final DimensionSelector dimSelector = cursor.makeDimensionSelector(
        query.getDimensionSpec()
    );    }
}

@himanshug
Copy link
Copy Markdown
Contributor Author

@binlijin that cardinality is only used to choose an algorithm, buffer I'm talking about is at https://github.com/druid-io/druid/blob/master/processing/src/main/java/io/druid/query/topn/PooledTopNAlgorithm.java#L67

@binlijin
Copy link
Copy Markdown
Contributor

@himanshug thanks, i am just want to make sure there is no problem, because i am not familiar with all logic.

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.

line55 could be:

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

@guobingkun
Copy link
Copy Markdown
Contributor

👍

fjy added a commit that referenced this pull request Jan 15, 2016
Remap id's returned in XXXFilteredDimensionSpec.getRow() as per reduced cardinality
@fjy fjy merged commit e0932ba into apache:master Jan 15, 2016
@fjy fjy added this to the 0.9.0 milestone Feb 4, 2016
@fjy fjy added the Improvement label Feb 5, 2016
@himanshug himanshug deleted the fix_topn_multi_val_filter branch February 8, 2016 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants