Skip to content
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

Support querying a data stream #56

Merged
merged 1 commit into from
May 18, 2021

Conversation

ketanv3
Copy link
Contributor

@ketanv3 ketanv3 commented May 14, 2021

Description

Fixed index expression resolution to include backing indices of a data stream. This will make querying a data stream consistent with querying an index alias.

Issues Resolved

Check List

  • Commits are signed per the DCO using --signoff

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.

Signed-off-by: Ketan Verma ketan9495@gmail.com

@dai-chen
Copy link
Collaborator

Thanks for contributing this PR! Could you fix the unit test error in Github action?
Btw, data stream seems a x-pack feature. Are you using x-pack with our SQL plugin together?

2021-05-14T15:44:56.3441278Z com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.mapping.FieldMappingsTest > flatFieldMappingsShouldIncludeFieldsOnAllLevels FAILED
2021-05-14T15:44:56.3444598Z     java.lang.IllegalStateException: Failed to read mapping in cluster state for indices=[field_mappings], types=[]
2021-05-14T15:44:56.3448366Z         at com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.LocalClusterState.getFieldMappings(LocalClusterState.java:218)
2021-05-14T15:44:56.3457993Z         at com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.LocalClusterState.getFieldMappings(LocalClusterState.java:172)
2021-05-14T15:44:56.3465882Z         at com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.mapping.FieldMappingsTest.flatFieldMappingsShouldIncludeFieldsOnAllLevels(FieldMappingsTest.java:68)
2021-05-14T15:44:56.3471283Z 
2021-05-14T15:44:56.3471639Z         Caused by:
2021-05-14T15:44:56.3472259Z         java.lang.NullPointerException
2021-05-14T15:44:56.3473122Z             at java.base/java.util.Arrays.sort(Arrays.java:1040)
2021-05-14T15:44:56.3475761Z             at com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.LocalClusterState.sortToList(LocalClusterState.java:251)
2021-05-14T15:44:56.3481108Z             at com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.LocalClusterState.findMappingsInCache(LocalClusterState.java:244)
2021-05-14T15:44:56.3486622Z             at com.amazon.opendistroforelasticsearch.sql.legacy.esdomain.LocalClusterState.getFieldMappings(LocalClusterState.java:206)
2021-05-14T15:44:56.3489248Z             ... 2 more

@ketanv3
Copy link
Contributor Author

ketanv3 commented May 14, 2021

@dai-chen Fixed the broken unit-tests.

We are adding data streams' support in OpenSearch which is being tracked here: opensearch-project/OpenSearch#675
We are also making sure that existing plugins are compatible with data streams once it's released.

@ketanv3 ketanv3 marked this pull request as ready for review May 14, 2021 17:46
@dai-chen dai-chen added the enhancement New feature or request label May 14, 2021
Copy link
Collaborator

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

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

@dai-chen
Copy link
Collaborator

Sorry I just realized that my PR renaming all packages to org.opensearch impacts yours. Could you please address the conflicts or rebase your changes?

…a stream.

Signed-off-by: Ketan Verma <ketan9495@gmail.com>
@ketanv3
Copy link
Contributor Author

ketanv3 commented May 18, 2021

@dai-chen No worries! I have rebased with the 'develop' branch.

@dai-chen dai-chen merged commit 288502f into opensearch-project:develop May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants