#639: allow metadata fields and score opensearch function (#228) by acarbonetto · Pull Request #1456 · opensearch-project/sql

acarbonetto · 2023-03-21T18:18:22Z

Description

OpenSearch reserved fields (_id, _index, _sort, _score, _max_score) are not allowed to be used in SQL clauses (SELECT, WHERE, ORDER BY) because the field format starting with underscore _ is not allowed.

This ticket adds specific identifiers to the language, and opens up support for OpenSearch reserved identifiers.
As an aside, identifiers with double underscore at the start (such as __myCoolField) is acceptable as an identifier.

This ticket allows for the score(), score_query() and scorequery() function to be used to wrap around relevance-search queries to force the _score and _max_score metadata fields to be returned with values. For some queries, _score returns null unless the score() function is included.

The score function also allows for an optional boost argument to be included that boosts the score of the child relevance function.

Example - metadata fields returned:

SELECT calcs.key, str0, _id, _sort, _score, _maxscore FROM calcs WHERE _id="5"
Result:
{ "key04", "OFFICE SUPPLIES", "5", -2, null, null }

Example - Metadata fields not requested are not displayed:

SELECT *, _id FROM bigint WHERE _id="2"
Result (assuming bigint only has one field):
[9223372026854775807, "2"]

Example - relevance search without and with score function

SELECT _id, _index, _score, _maxscore, str0, str1 FROM calcs WHERE QUERY_STRING([\"*\"], `BINDING SUPPLIES`);
result: 
[
    "7",
    "calcs",
    null,
    null,
    "OFFICE SUPPLIES",
    "BINDING SUPPLIES"
]

SELECT _id, _index, _score, _maxscore, str0, str1 FROM calcs WHERE SCORE(QUERY_STRING([\"*\"], `BINDING SUPPLIES`));
[
    "7",
    "calcs",
    2.4849067,
    2,
    "OFFICE SUPPLIES",
    "BINDING SUPPLIES"
]

Example - boost score on the _sql/_explain call:

SELECT _id, _score, _maxscore FROM calcs WHERE SCORE(QUERY_STRING([\"*\"], `BINDING SUPPLIES`, boost=2.5));
result: 
[
    "7",
    6.2122664,
    6
]

SELECT _id, _score, _maxscore FROM calcs WHERE SCOREQUERY(QUERY_STRING([\"*\"], `BINDING SUPPLIES`, boost=2.5), 2.0);
result: 
[
    "7",
    12.424533,
    12
]

SELECT _id, _index, _score, _maxscore, str0, str1 FROM calcs WHERE SCORE_QUERY(QUERY_STRING([\"*\"], `BINDING SUPPLIES`), 2.5);
result:
[
    "7",
    6.2122664,
    6
]

Issues Resolved

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
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.

…nction (#228) * Rebase from main Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update to define and include metadata when visiting the expr node Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Add specific metadata identifiers Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Add IT tests and add parser changes Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Rebase from main Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update score function expression analyzer to return boosted relevance function Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update builder to track scores Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Remove ScoreExpression.java and cleanup checkstyle Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * cleanup checkstyle Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Cleanup and add alternative score function syntax Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Cleanup and add alternative score function syntax Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Fix some bugs and add Expression tests Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Add expresssion and analyzer tests Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Add score doctests Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Add score function doctests Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Add metafield tests Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Move legacy test and mark old as ignore Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * fix checkstyle violations Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * fix checkstyle violations Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update tests and identifier to accept metafields Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Checkstyle fixes Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Rebase from main Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Rebase from main Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Rebase from main Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * fix checkstyle violations Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Revert bad conflict resolution Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Fix for review comments Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update IT tests and legacy tests for comments Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Minor comment Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Updates for whitespace Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update basics.rst to show OS result Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update basics.rst to show OS result Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update basics.rst description Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Change Score function to accept a double/integer not an unresolved Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update functions.rst Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Checkstyle update Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Move reserved world symbol table to OpenSearchTable Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Update functions.rst for review comments Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Removed parser meta tokens; Changes ImmutableMap to Map Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> * Removed parser meta tokens; Changes ImmutableMap to Map Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> --------- Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

codecov-commenter · 2023-03-21T18:39:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.49%. Comparing base (23cc0f6) to head (05c42f7).
⚠️ Report is 1034 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1456      +/-   ##
============================================
+ Coverage     98.46%   98.49%   +0.02%     
- Complexity     3869     3928      +59     
============================================
  Files           345      347       +2     
  Lines          9603     9771     +168     
  Branches        616      645      +29     
============================================
+ Hits           9456     9624     +168     
  Misses          142      142              
  Partials          5        5

Flag	Coverage Δ
sql-engine	`98.49% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

MaxKsyunz

@acarbonetto Do metafields work with aliases? E.g. SELECT h._id FROM tableA AS h

Is there a test that confirms this?

Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

acarbonetto · 2023-03-22T23:40:58Z

@acarbonetto Do metafields work with aliases? E.g. SELECT h._id FROM tableA AS h

Is there a test that confirms this?

IT test for this added in f3d89b8

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

dai-chen · 2023-03-28T18:09:24Z

sql/src/main/antlr/OpenSearchSQLLexer.g4

+// Identifiers cannot start with a single '_' since this an OpenSearch reserved
+// metadata field.  Two underscores (or more) is acceptable, such as '__field'.


Does this comment match the rule below? So ID_LITERAL won't accept token starting with single underscore?

Good catch! I need to update this comment since I've updated the pattern.
Thank you!

opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

core/src/main/java/org/opensearch/sql/analysis/ExpressionReferenceOptimizer.java

Yury-Fridlyand · 2023-04-06T19:15:22Z

opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java

+    FetchSourceContext fetchSource = this.sourceBuilder.fetchSource();
+    List<String> includes = fetchSource != null && fetchSource.includes() != null
+            ? Arrays.asList(fetchSource.includes())
+            : List.of();


May be do this in the query builder?

OpenSearchQueryRequest and OpenSearchScrollRequest needs a refactor before this makes any sense.

Please assist in creating an issue to monitor and manage future modifications.

opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java

opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java

Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

…ction (#228) (#1456) Allow metadata fields and score OpenSearch function. Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> (cherry picked from commit e805151)

…ion (#228) (#1509) * #639: Support OpenSearch metadata fields and the score OpenSearch function (#228) (#1456) Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com>

…ion (#228) (#1508) * #639: Support OpenSearch metadata fields and the score OpenSearch function (#228) (#1456) Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com>

…elds and the score OpenSearch function (#228) (opensearch-project#1456) Allow metadata fields and score OpenSearch function. Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

acarbonetto requested a review from a team as a code owner March 21, 2023 18:18

Yury-Fridlyand added new-engine-gap backport 2.x labels Mar 21, 2023

Yury-Fridlyand requested review from dai-chen and penghuo March 21, 2023 18:41

MaxKsyunz previously approved these changes Mar 21, 2023

View reviewed changes

MaxKsyunz reviewed Mar 22, 2023

View reviewed changes

Add IT test with meta-fields and aliases

f3d89b8

Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

acarbonetto dismissed MaxKsyunz’s stale review via f3d89b8 March 22, 2023 23:39

acarbonetto requested review from YANG-DB, Yury-Fridlyand, anirudha, derek-ho, joshuali925, kavithacm, mengweieric, ps48, rupal-bq and vamsimanohar as code owners March 22, 2023 23:40

GumpacG previously approved these changes Mar 23, 2023

View reviewed changes

acarbonetto mentioned this pull request Mar 24, 2023

SQL ingestion and ops - standup #791

Closed

GabeFernandez310 previously approved these changes Mar 24, 2023

View reviewed changes

Yury-Fridlyand previously approved these changes Mar 25, 2023

View reviewed changes

MaxKsyunz previously approved these changes Mar 27, 2023

View reviewed changes

matthewryanwells previously approved these changes Mar 27, 2023

View reviewed changes

penghuo reviewed Mar 28, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java Outdated Show resolved Hide resolved

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java Show resolved Hide resolved

dai-chen reviewed Mar 28, 2023

View reviewed changes

MaxKsyunz added the backport 2.7 label Apr 6, 2023

Yury-Fridlyand previously approved these changes Apr 6, 2023

View reviewed changes

Update to use set for building metafields in response

ccdc290

Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

acarbonetto dismissed stale reviews from Yury-Fridlyand, MaxKsyunz, and GabeFernandez310 via ccdc290 April 6, 2023 21:25

Update tests and checkstyle

05c42f7

Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com>

Yury-Fridlyand approved these changes Apr 6, 2023

View reviewed changes

GabeFernandez310 approved these changes Apr 7, 2023

View reviewed changes

penghuo mentioned this pull request Apr 10, 2023

[Enhancement] Refactor reservedSymbolTable table. #1506

Closed

penghuo approved these changes Apr 10, 2023

View reviewed changes

MaxKsyunz approved these changes Apr 10, 2023

View reviewed changes

MaxKsyunz merged commit e805151 into opensearch-project:main Apr 10, 2023

MaxKsyunz deleted the integ-metadata-fields branch April 10, 2023 19:42

opensearch-trigger-bot bot mentioned this pull request Apr 10, 2023

[Backport 2.x] #639: allow metadata fields and score opensearch function (#228) #1508

Merged

opensearch-trigger-bot bot mentioned this pull request Apr 10, 2023

[Backport 2.7] #639: allow metadata fields and score opensearch function (#228) #1509

Merged

parked-toes mentioned this pull request Aug 1, 2023

[FEATURE] Retrieve the original raw document via SQL (_source column) #1918

Closed

This was referenced Jun 28, 2024

[BUG] _id is not available in PPL where condition #2788

Closed

Allow metadata fields in PPL query #2789

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#639: allow metadata fields and score opensearch function (#228)#1456

#639: allow metadata fields and score opensearch function (#228)#1456
MaxKsyunz merged 9 commits intoopensearch-project:mainfrom
Bit-Quill:integ-metadata-fields

acarbonetto commented Mar 21, 2023

Uh oh!

codecov-commenter commented Mar 21, 2023 •

edited by codecov bot

Loading

Uh oh!

MaxKsyunz left a comment

Uh oh!

acarbonetto commented Mar 22, 2023

Uh oh!

Uh oh!

Uh oh!

dai-chen Mar 28, 2023

Uh oh!

acarbonetto Mar 29, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yury-Fridlyand Apr 6, 2023

Uh oh!

acarbonetto Apr 6, 2023

Uh oh!

penghuo Apr 10, 2023

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

		// Identifiers cannot start with a single '_' since this an OpenSearch reserved
		// metadata field. Two underscores (or more) is acceptable, such as '__field'.

Conversation

acarbonetto commented Mar 21, 2023

Description

Issues Resolved

Check List

Uh oh!

codecov-commenter commented Mar 21, 2023 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MaxKsyunz left a comment

Choose a reason for hiding this comment

Uh oh!

acarbonetto commented Mar 22, 2023

Uh oh!

Uh oh!

Uh oh!

dai-chen Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

acarbonetto Mar 29, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yury-Fridlyand Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

acarbonetto Apr 6, 2023

Choose a reason for hiding this comment

Uh oh!

penghuo Apr 10, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

codecov-commenter commented Mar 21, 2023 •

edited by codecov bot

Loading