OpenSearch icon indicating copy to clipboard operation
OpenSearch copied to clipboard

Extract all used fields from QueryStringQueryBuilder

Open petardz opened this issue 3 years ago • 20 comments

Signed-off-by: Petar Dzepina [email protected]

Description

We recently implemented rewriting of fields used in QueryStringQueryBuilder in ISM and we had to copy over substantial amount of code from core in ISM. To avoid this code duplication and future maintenance, we could extend core's QueryStringQueryBuilder to provide method for extracting all fields which are used in final query.

Also in Alerting we would have use-case for this, where if we could grab all fields used in queries, we could just copy mappings to percolate index only for those fields.

Issues Resolved

#6019

Check List

  • [x] New functionality includes testing.
    • [x] All tests pass
  • [ ] New functionality has been documented.
    • [ ] New functionality has javadoc added
  • [x] Commits are signed per the DCO using --signoff
  • [ ] Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

petardz avatar Jan 26 '23 01:01 petardz

Gradle Check (Jenkins) Run Completed with:

  • RESULT: SUCCESS :white_check_mark:
  • URL: https://build.ci.opensearch.org/job/gradle-check/9939/
  • CommitID: 4060d0c7b65602b3f5a2668ec4dfbb57b46097ad

github-actions[bot] avatar Jan 26 '23 01:01 github-actions[bot]

Codecov Report

Attention: Patch coverage is 78.26087% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 70.84%. Comparing base (e265355) to head (8738485). Report is 426 commits behind head on main.

:exclamation: Current head 8738485 differs from pull request most recent head 2616db2. Consider uploading reports for the commit 2616db2 to get more accurate results

Files Patch % Lines
...pensearch/index/query/QueryStringQueryBuilder.java 56.25% 4 Missing and 3 partials :warning:
...pensearch/index/search/QueryStringQueryParser.java 90.00% 2 Missing and 1 partial :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6020      +/-   ##
============================================
- Coverage     71.41%   70.84%   -0.57%     
+ Complexity    59397    58760     -637     
============================================
  Files          4923     4771     -152     
  Lines        279212   280863    +1651     
  Branches      40595    40571      -24     
============================================
- Hits         199408   198989     -419     
- Misses        63223    65557    +2334     
+ Partials      16581    16317     -264     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Jan 26 '23 01:01 codecov-commenter

Gradle Check (Jenkins) Run Completed with:

  • RESULT: SUCCESS :white_check_mark:
  • URL: https://build.ci.opensearch.org/job/gradle-check/9940/
  • CommitID: 4727bb28330da54a4b03cfb3594759a2ecb7d3f8

github-actions[bot] avatar Jan 26 '23 01:01 github-actions[bot]

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE :grey_exclamation:
  • TEST FAILURES:
      1 org.opensearch.search.basic.SearchWithRandomIOExceptionsIT.testRandomDirectoryIOExceptions
      1 org.opensearch.search.basic.SearchWithRandomIOExceptionsIT.classMethod
      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockWithAReadOnlyBlock
      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermark
  • URL: https://build.ci.opensearch.org/job/gradle-check/9955/
  • CommitID: 54f91751c5524579119d928f50a4aab02393ad85 Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

github-actions[bot] avatar Jan 26 '23 16:01 github-actions[bot]

Gradle Check (Jenkins) Run Completed with:

  • RESULT: FAILURE :x:
  • URL: https://build.ci.opensearch.org/job/gradle-check/9998/
  • CommitID: c3379b789781fc758cdb5563adcf68515e25cfd4 Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions[bot] avatar Jan 26 '23 22:01 github-actions[bot]

Thanks @petardz LGTM, @nknize do you have time to take a look? thank you

reta avatar Jan 27 '23 00:01 reta

Gradle Check (Jenkins) Run Completed with:

  • RESULT: SUCCESS :white_check_mark:
  • URL: https://build.ci.opensearch.org/job/gradle-check/10000/
  • CommitID: 873848545618d317c76bf8ccb91895fe46f64d68

github-actions[bot] avatar Jan 27 '23 00:01 github-actions[bot]

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

This PR was closed because it has been stalled for 7 days with no activity.

Apologies. This PR was auto closed without reaching a resolution from the maintainers. Re-opening to move it forward. Thanks for your contributions to OpenSearch!

kotwanikunal avatar Sep 14 '23 18:09 kotwanikunal

Compatibility status:

Checks if related components are compatible with change 2616db2

Incompatible components

Incompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer-rca.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/k-nn.git]

github-actions[bot] avatar Sep 14 '23 18:09 github-actions[bot]

Gradle Check (Jenkins) Run Completed with:

  • RESULT: FAILURE :x:
  • URL: https://build.ci.opensearch.org/job/gradle-check/25637/
  • CommitID: 873848545618d317c76bf8ccb91895fe46f64d68 Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions[bot] avatar Sep 14 '23 19:09 github-actions[bot]

@petardz Is this being worked upon? Pls free to reach out to maintainers for further reviews.

ashking94 avatar Dec 26 '23 08:12 ashking94

Hi @petardz, do we have any updates?

ticheng-aws avatar Jan 05 '24 23:01 ticheng-aws

This was gonna be useful for ISM plugin for Rollup feature where field names are getting rewritten. Currently, ISM is copying over bunch of code from QSQ implementation from core.

Also, this could be useful for Alerting plugin too. Currently, Alerting is copying over mappings from source index to query index for ALL fields, but it could be copying only ones used in percolate queries, if parsing query fields would be trivial.

@bowenlan-amzn @sbcd90 @praveensameneni @getsaurabh02 if you still find this useful I could rebase this PR.

petardz avatar Jan 06 '24 00:01 petardz

@sbcd90 @praveensameneni I belive Subho worked on this before, can do a review. @petardz please rebase and let's get this in, don't want the effort here to be wasted.

bowenlan-amzn avatar Jan 06 '24 02:01 bowenlan-amzn

:x: Gradle check result for 2616db2015bf94ada61fe7f6f51493ea5144d43b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions[bot] avatar Jan 22 '24 01:01 github-actions[bot]

It seems like this contribution is stalled. I would recommend we close it for now. @peternied could you close this for us? Thanks

stephen-crawford avatar Mar 29 '24 20:03 stephen-crawford

Thanks for the mention @scrawfor99 but this is a little outside my experience.

@eirsep @sbcd90 @praveensamenen @bowenlan-amzn Maybe one of you could weight in on what you think should happen next for this PR? Is this something one of you would want to drive to completion?

peternied avatar Apr 09 '24 20:04 peternied

This PR is stalled because it has been open for 30 days with no activity.

Closing, please feel free to finish and reopen. [Catch All Triage - 1, 2, 3, 4]

dblock avatar Jul 15 '24 16:07 dblock