Allow SegmentMetadataQuery to skip cardinality and size calculations #1753
Conversation
|
fixes #1750 |
| @@ -29,6 +29,7 @@ There are several main parts to a segment metadata query: | |||
| |toInclude|A JSON Object representing what columns should be included in the result. Defaults to "all".|no| | |||
| |merge|Merge all individual segment metadata results into a single result|no| | |||
| |context|See [Context](../querying/query-context.html)|no| | |||
| |analysisTypes|A list of Strings specifying what column properties (e.g. cardinality, size) should be calculated and returned in the result. Defaults to ["CARDINALITY", "SIZE"]. If cardinality and/or size values are not needed, omitting those properties will result in a more efficient query.|no| | |||
There was a problem hiding this comment.
I would prefer if we had consistent casing in our query APIs and default everything to lower / camelCase. Maybe we can do something to have better serde for enums?
There was a problem hiding this comment.
@xvrl I added a fromString function to the analysisTypes enum that will uppercase the input, changed the docs to use lowercase analysisType strings
jon-wei
force-pushed
the
segmentmetadataquery_flags
branch
from
3fcb4b8
to
e591d7b
Compare
|
👍 |
| @@ -55,7 +56,7 @@ | |||
| */ | |||
| private static final int NUM_BYTES_IN_TEXT_FLOAT = 8; | |||
|
|
|||
| public Map<String, ColumnAnalysis> analyze(QueryableIndex index) | |||
| public Map<String, ColumnAnalysis> analyze(QueryableIndex index, SegmentMetadataQuery query) | |||
There was a problem hiding this comment.
Rather than passing the query around, can you pass the flag set around? I think it's nicer to pass smaller things, when possible.
jon-wei
force-pushed
the
segmentmetadataquery_flags
branch
from
334b0ca
to
b6468e5
Compare
| @@ -29,6 +29,7 @@ There are several main parts to a segment metadata query: | |||
| |toInclude|A JSON Object representing what columns should be included in the result. Defaults to "all".|no| | |||
| |merge|Merge all individual segment metadata results into a single result|no| | |||
| |context|See [Context](../querying/query-context.html)|no| | |||
| |analysisTypes|A list of Strings specifying what column properties (e.g. cardinality, size) should be calculated and returned in the result. Defaults to ["cardinality", "size"]. If cardinality and/or size values are not needed, omitting those properties will result in a more efficient query.|no| | |||
There was a problem hiding this comment.
I think this should be a link to a section of the segment metadata query listing out the analysis types and what each one does. There's enough info there that it is worth expanding on what's here, but that would take up too much space in the table.
jon-wei
force-pushed
the
segmentmetadataquery_flags
branch
3 times, most recently
from
8f622f7
to
a716d17
Compare
| @@ -86,3 +87,21 @@ The grammar is as follows: | |||
| ``` json | |||
| "toInclude": { "type": "list", "columns": [<string list of column names>]} | |||
| ``` | |||
|
|
|||
| ### analysisTypes <a name="analysisTypes"></a> | |||
There was a problem hiding this comment.
no need for anchor here, our markdown processor automatically adds them
There was a problem hiding this comment.
FYI, markdown creates all lowercase anchors, so you can refer to it using analysistypes, similar to http://druid.io/docs/latest/querying/segmentmetadataquery.html#toinclude
There was a problem hiding this comment.
@xvrl removed anchor
|
LGTM, some minor comments, but good to merge once we remove unnecessary doc anchors. |
jon-wei
force-pushed
the
segmentmetadataquery_flags
branch
from
a716d17
to
96bce13
Compare
| @@ -29,6 +29,7 @@ There are several main parts to a segment metadata query: | |||
| |toInclude|A JSON Object representing what columns should be included in the result. Defaults to "all".|no| | |||
| |merge|Merge all individual segment metadata results into a single result|no| | |||
| |context|See [Context](../querying/query-context.html)|no| | |||
| |analysisTypes|A list of Strings specifying what column properties (e.g. cardinality, size) should be calculated and returned in the result. Defaults to ["cardinality", "size"]. See section [analysisTypes](#analysisTypes) for more details.|no| | |||
There was a problem hiding this comment.
oops, just saw this, link needs to be lowercase [analysisTypes](#analysistypes)
There was a problem hiding this comment.
@xvrl Lowercased the link
jon-wei
force-pushed
the
segmentmetadataquery_flags
branch
from
96bce13
to
e6a6284
Compare
|
👍 |
Allow SegmentMetadataQuery to skip cardinality and size calculations
No description provided.