-
Notifications
You must be signed in to change notification settings - Fork 23
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.
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
Add describe and show covering index SQL support #36
Add describe and show covering index SQL support #36
Conversation
Signed-off-by: Chen Dai <daichen@amazon.com>
Signed-off-by: Chen Dai <daichen@amazon.com>
Signed-off-by: Chen Dai <daichen@amazon.com>
Signed-off-by: Chen Dai <daichen@amazon.com>
bd3f053
to
3c77a98
Compare
...c/main/scala/org/opensearch/flint/spark/sql/covering/FlintSparkCoveringIndexAstBuilder.scala
Show resolved
Hide resolved
* @param indexNamePattern index name pattern | ||
* @return all matched index metadata | ||
*/ | ||
List<FlintMetadata> getAllIndexMetadata(String indexNamePattern); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the 'All' needed ? the parameter nane and return value already indicate this is a pattern and may return a list of indices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because there is another API FlintMetadata getIndexMetadata(String indexName)
below with same name and argument list. I'm thinking get[All]IndexMetadata
maybe better than getIndexMetadata[s]
?
I was also thinking of providing single getIndexMetadata API for both purpose. But it seems confusing when we pass index name and expect single metadata returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe getIndicesMetadata
- anyhow the return signature also helps distinguish between them IMO ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can rename it to getIndexMetadata[s]
if you have strong opinion? Or any better name? (getIndexMetadataWithPattern?)
IMO, it's intuitive to have different method name and also required by compiler... Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will merge this for now and rename if we come up with better name. Thanks!
Description
DESC INDEX
statement supportSHOW INDEX
index statement support based on a newgetAllIndexMetadata()
API in Flint clientExample
Issues Resolved
#23
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.