Skip to content

Support min/max values for metadata query#2208

Merged
jon-wei merged 1 commit intoapache:masterfrom
navis:metadataquery-minmax
Feb 12, 2016
Merged

Support min/max values for metadata query#2208
jon-wei merged 1 commit intoapache:masterfrom
navis:metadataquery-minmax

Conversation

@navis
Copy link
Copy Markdown
Contributor

@navis navis commented Jan 6, 2016

Support AnalysisType.MIN_MAX for metadata queries. Currently it's supported only for string dimension with bitmap index.

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.

since you have this open, please make this say "return per-segment information"?

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.

ok

@navis navis force-pushed the metadataquery-minmax branch from 135aca9 to 0b5f5ca Compare January 7, 2016 01:31
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typically we have use camel-case for any constants in the query API

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.

got it.

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.

@xvrl The json value for AnalysisType is name().toLowerCase(). so I've changed it to minmax for consistency (not minMax). Would it be ok?

@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Jan 9, 2016

I'm also wondering why the hashCode() function for SegmentAnalysis changed, but 👍 otherwise

@navis navis force-pushed the metadataquery-minmax branch from 0b5f5ca to 0139496 Compare January 11, 2016 00:52
@navis
Copy link
Copy Markdown
Contributor Author

navis commented Jan 20, 2016

Test failed by regression of #2006

@navis navis force-pushed the metadataquery-minmax branch from 0139496 to 0c144b6 Compare January 26, 2016 00:52
@navis
Copy link
Copy Markdown
Contributor Author

navis commented Jan 26, 2016

Rebased on master. Now incremental index also returns estimated size.

@navis navis force-pushed the metadataquery-minmax branch from 0c144b6 to d474eb6 Compare January 26, 2016 01:14
@navis navis force-pushed the metadataquery-minmax branch 2 times, most recently from 108cfa0 to 37fdca7 Compare February 4, 2016 02:20
@fjy fjy modified the milestone: 0.9.1 Feb 5, 2016
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.

can we add a serde test checking for this?

@fjy fjy added the Feature label Feb 6, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Feb 9, 2016

@navis any chance of finishing this up?

@navis navis force-pushed the metadataquery-minmax branch from 37fdca7 to bb6c912 Compare February 10, 2016 10:03
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Feb 11, 2016

👍 after commits squashed

@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Feb 11, 2016

👍 after squash

@navis navis force-pushed the metadataquery-minmax branch from bb6c912 to dd23754 Compare February 12, 2016 00:36
@navis
Copy link
Copy Markdown
Contributor Author

navis commented Feb 12, 2016

Not yet decided if we remove size in the metadataquery but squashed anyway.

jon-wei added a commit that referenced this pull request Feb 12, 2016
Support min/max values for metadata query
@jon-wei jon-wei merged commit d63eec6 into apache:master Feb 12, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Feb 12, 2016

@navis we probably should wait until 0.10.0 to remove size, but I agree with Gian what it should actually return is the actual on disk size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants