[Auto Suggest] Add back reverted MDS support, tests and cleanup for sql auto suggest #7543

mengweieric · 2024-07-28T00:56:33Z

Description

Cherry-pick MDS support for sql autosuggest
Tests update
Minor cleanup

Issues Resolved

Screenshot

Testing the changes

Changelog

feat: [Auto Suggest] Add MDS support, tests and cleanup for sql auto suggest

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

github-actions · 2024-07-28T00:57:06Z

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

codecov · 2024-07-28T01:10:32Z

Codecov Report

Attention: Patch coverage is 51.42857% with 17 lines in your changes missing coverage. Please review.

Project coverage is 63.68%. Comparing base (611f56b) to head (9e7b1be).
Report is 1 commits behind head on main.

Files	Patch %	Lines
...ata/public/antlr/opensearch_sql/code_completion.ts	0.00%	16 Missing ⚠️
src/plugins/data/public/antlr/shared/utils.ts	94.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7543      +/-   ##
==========================================
+ Coverage   63.64%   63.68%   +0.03%     
==========================================
  Files        3629     3629              
  Lines       79520    79516       -4     
  Branches    12601    12597       -4     
==========================================
+ Hits        50611    50640      +29     
+ Misses      25841    25804      -37     
- Partials     3068     3072       +4

Flag	Coverage Δ
Linux_1	`30.76% <5.71%> (+<0.01%)`	⬆️
Linux_2	`55.58% <ø> (ø)`
Linux_3	`40.38% <51.42%> (+0.15%)`	⬆️
Linux_4	`31.57% <5.71%> (+<0.01%)`	⬆️
Windows_1	`30.78% <5.71%> (+<0.01%)`	⬆️
Windows_2	`55.53% <ø> (ø)`
Windows_3	`40.38% <51.42%> (+0.15%)`	⬆️
Windows_4	`31.57% <5.71%> (+<0.01%)`	⬆️

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.

github-actions · 2024-07-28T02:28:54Z

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

Signed-off-by: Kawika Avilla <[email protected]> almost working pretty nicely Signed-off-by: Kawika Avilla <[email protected]> a little bit better Signed-off-by: Kawika Avilla <[email protected]> its ok Signed-off-by: Kawika Avilla <[email protected]>

Signed-off-by: Kawika Avilla <[email protected]>

…ensearch-project#7463) * add tests for sql autocomplete rule processing Signed-off-by: Eric <[email protected]> * refer to monaco type directly Signed-off-by: Eric <[email protected]> * remove unnecessary antlr auto generated files Signed-off-by: Eric <[email protected]> * inital adoption of dataSet manager Signed-off-by: Eric <[email protected]> * mds support Signed-off-by: Eric <[email protected]> * remove test that are failed due to adopting dataSet manager Signed-off-by: Eric <[email protected]> * add changelog Signed-off-by: Eric <[email protected]> * fix(query assist): update reading data source id from dataset manager (opensearch-project#7464) * revert to read datasource id from index pattern Signed-off-by: Joshua Li <[email protected]> * add dataset mock to query mock Signed-off-by: Joshua Li <[email protected]> * update query assist to use dataset manager Signed-off-by: Joshua Li <[email protected]> * use selected dataset state instead of relying on rerender Signed-off-by: Joshua Li <[email protected]> * remove skip 1 in dataset observable Signed-off-by: Joshua Li <[email protected]> * update dataset_manager tests Signed-off-by: Joshua Li <[email protected]> --------- Signed-off-by: Joshua Li <[email protected]> * update utils Signed-off-by: Eric <[email protected]> * keep with observable and remove values suggestion Signed-off-by: Eric <[email protected]> * update unit tests Signed-off-by: Eric <[email protected]> * [Auto Suggest] DQL autosuggest with ANTLR (opensearch-project#7467) * Antlr autocomplete (opensearch-project#7159) * dql grammar with rudamentary testing parser Signed-off-by: Paul Sebastian <[email protected]> * show suggestion of fields depending on current index pattern Signed-off-by: Paul Sebastian <[email protected]> * basic code completion with fields populated Signed-off-by: Paul Sebastian <[email protected]> * updated grammar and generated for better group handling Signed-off-by: Paul Sebastian <[email protected]> * add ignored tokens Signed-off-by: Paul Sebastian <[email protected]> * remove console logs Signed-off-by: Paul Sebastian <[email protected]> --------- Signed-off-by: Paul Sebastian <[email protected]> * dql Antlr autocomplete (opensearch-project#7160) * re-add provider for sql Signed-off-by: Paul Sebastian <[email protected]> * added temporary fix for language providor to appear for more than one language Signed-off-by: Paul Sebastian <[email protected]> --------- Signed-off-by: Paul Sebastian <[email protected]> * remove EOF in parser to fix suggestions Signed-off-by: Paul Sebastian <[email protected]> * use custom version of cursor token index for dql Signed-off-by: Paul Sebastian <[email protected]> * implemented value suggestions based on field Signed-off-by: Paul Sebastian <[email protected]> * set param type Signed-off-by: Paul Sebastian <[email protected]> * update grouping grammar Signed-off-by: Paul Sebastian <[email protected]> * fix grammar for dots in field and value term search with spaces Signed-off-by: Paul Sebastian <[email protected]> * value suggestions match field to avoid failing api call and to find assc keyword field Signed-off-by: Paul Sebastian <[email protected]> * update value suggestions from partially formed value Signed-off-by: Paul Sebastian <[email protected]> * refactor value suggestions and change fieldval listener to visitor Signed-off-by: Paul Sebastian <[email protected]> * implement value suggestions within phrases Signed-off-by: Paul Sebastian <[email protected]> * make grammar more readable Signed-off-by: Paul Sebastian <[email protected]> * rename grammar parser rules Signed-off-by: Paul Sebastian <[email protected]> * bring back minimal autocomplete optimized grammar Signed-off-by: Paul Sebastian <[email protected]> * enable partially complete value suggestion for value groups Signed-off-by: Paul Sebastian <[email protected]> * remove number as lexer rule Signed-off-by: Paul Sebastian <[email protected]> * fix cursor import and clean up Signed-off-by: Paul Sebastian <[email protected]> * fix completion item range to be current word Signed-off-by: Paul Sebastian <[email protected]> * update cursor to use monaco position Signed-off-by: Paul Sebastian <[email protected]> * cursor index to use position directly Signed-off-by: Paul Sebastian <[email protected]> * move language registration into render function to handle new languages Signed-off-by: Paul Sebastian <[email protected]> * include auto closing quotes and parenthesis for dql Signed-off-by: Paul Sebastian <[email protected]> * rename generated file Signed-off-by: Paul Sebastian <[email protected]> * include single line editor closing pairs Signed-off-by: Paul Sebastian <[email protected]> * Changeset file for PR opensearch-project#7391 created/updated * add license and fix linting Signed-off-by: Paul Sebastian <[email protected]> * modify grammar Signed-off-by: Paul Sebastian <[email protected]> * add tests for fields and keywords Signed-off-by: Paul Sebastian <[email protected]> * move dql test constants to separate file Signed-off-by: Paul Sebastian <[email protected]> * pass core setup from autocomplete constructor to query sugg provider and utilize selectionEnd if no position Signed-off-by: Paul Sebastian <[email protected]> * update an import Signed-off-by: Paul Sebastian <[email protected]> * use updated dataset for index pattern Signed-off-by: Paul Sebastian <[email protected]> * remove console log Signed-off-by: Paul Sebastian <[email protected]> --------- Signed-off-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> * [tests][discover-next] update the tests and async nature of the dataset navigator (opensearch-project#7489) * [tests][discover-next] update the tests and async nature of the dataset manager Address test failures related to the dataset navigator. Signed-off-by: Kawika Avilla <[email protected]> * bad fingers accidentally hit the x button Signed-off-by: Kawika Avilla <[email protected]> --------- Signed-off-by: Kawika Avilla <[email protected]> * resolve conflicts Signed-off-by: Eric <[email protected]> * fix one minor linting Signed-off-by: Eric <[email protected]> --------- Signed-off-by: Eric <[email protected]> Signed-off-by: Joshua Li <[email protected]> Signed-off-by: Paul Sebastian <[email protected]> Signed-off-by: Kawika Avilla <[email protected]> Signed-off-by: Eric Wei <[email protected]> Co-authored-by: Joshua Li <[email protected]> Co-authored-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Kawika Avilla <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]>

Signed-off-by: Eric <[email protected]>

Signed-off-by: Eric Wei <[email protected]>

.gitignore

src/plugins/data/public/ui/query_editor/editors/default_editor/index.tsx

kavilla · 2024-07-30T05:49:26Z

Some questions that if you have some insight could unblock/give peace of mind.

does autocomplete values display sensitive info and filter results if I guess the letters until I get a single value [question in DQL]
does SQL fire a request per key stroke? if so we should debounce [question in DQL]

mengweieric · 2024-07-30T11:49:04Z

Some questions that if you have some insight could unblock/give peace of mind.

does autocomplete values display sensitive info and filter results if I guess the letters until I get a single value [question in DQL]

does SQL fire a request per key stroke? if so we should debounce [question in DQL]

Thanks for review.

This SQL implementation does not contain values suggestion as it is not a common practice to suggest values with a few concerns including the security issue you mentioned.

As respect to your second question if there's field match then no additional request is sent, but ideally the plan was to also leverage both index patterns and cache layer together maybe so that if there's no dataset changes the schema is only fetched once for all the following field suggestion. I believe as queryEnhancement and data plugin with MQL features are getting into a good shape, we should be able to leverage more core APIs from these plugins to fetch schema and manage request in a more optimal way.

ashwin-pc

Approving given the response.

changelogs/fragments/7463.yml

src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

joshuali925 · 2024-07-30T16:46:47Z

src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

        }))
      );
    }
  } catch (error) {
-    // TODO: pipe error to the UI
+    // TODO: Handle errors appropriately, possibly logging or displaying a message to the user
+    return [];


is this catching errors for await fetchTableSchemas or something else could also throw error? if the network call fails, do we want to show other suggestions like suggestAggregateFunctions if possible?

Yea we could but the only concern here is if fetching the schema is having error, but user is still getting other suggestions, would this error being potentially hidden in this case that user still thinks everything is working accurately?

i would think that there are different completion sources, one source failing shouldn't affect others. the TODO Handle errors appropriately, possibly logging or displaying a message to the user should help to notify user about any failure

src/plugins/data/public/antlr/shared/utils.ts

joshuali925 · 2024-07-30T16:50:39Z

src/plugins/data/public/antlr/shared/utils.ts

    })
  );

+const fetchFromAPI = async (api: any, body: string) => {
+  try {
+    return await api.http.fetch({


what is api and why not just return api.http.fetch without try catch?

joshuali925 · 2024-07-30T16:53:39Z

src/plugins/data/public/antlr/shared/utils.ts

      }
    ).subscribe({
      next: (dataFrames: any) => resolve(dataFrames),
-      error: (err: any) => {
+      error: (err: Error) => {


seems like this resolves and rejects immediately and only once, do you need to make getRawSuggestionData an observable? and it would be better to avoid any for easier maintenance

…ql auto suggest (#7543) This PR did: * Added support for MDS (likely Multi-Data Source) in the auto-suggest feature. * Implemented DQL auto-suggest functionality using ANTLR (ANother Tool for Language Recognition). * Updated the query assist feature to use a dataset manager instead of relying on index patterns directly. * Improved field and value suggestions in DQL, including support for partially formed values and phrases. * Added auto-closing quotes and parentheses for DQL in the editor. * Updated the grammar and parser rules for better handling of DQL syntax. * Implemented tests for SQL autocomplete rule processing and DQL field and keyword suggestions. * Made changes to handle the asynchronous nature of the dataset manager in tests. * Cleaned up unnecessary files and updated utilities. --------- Signed-off-by: Kawika Avilla <[email protected]> Signed-off-by: Eric <[email protected]> Signed-off-by: Joshua Li <[email protected]> Signed-off-by: Paul Sebastian <[email protected]> Signed-off-by: Eric Wei <[email protected]> Co-authored-by: Kawika Avilla <[email protected]> Co-authored-by: Joshua Li <[email protected]> Co-authored-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Ashwin P Chandran <[email protected]> (cherry picked from commit d0618df) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…ql auto suggest (#7543) (#7579) This PR did: * Added support for MDS (likely Multi-Data Source) in the auto-suggest feature. * Implemented DQL auto-suggest functionality using ANTLR (ANother Tool for Language Recognition). * Updated the query assist feature to use a dataset manager instead of relying on index patterns directly. * Improved field and value suggestions in DQL, including support for partially formed values and phrases. * Added auto-closing quotes and parentheses for DQL in the editor. * Updated the grammar and parser rules for better handling of DQL syntax. * Implemented tests for SQL autocomplete rule processing and DQL field and keyword suggestions. * Made changes to handle the asynchronous nature of the dataset manager in tests. * Cleaned up unnecessary files and updated utilities. --------- (cherry picked from commit d0618df) Signed-off-by: Kawika Avilla <[email protected]> Signed-off-by: Eric <[email protected]> Signed-off-by: Joshua Li <[email protected]> Signed-off-by: Paul Sebastian <[email protected]> Signed-off-by: Eric Wei <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Kawika Avilla <[email protected]> Co-authored-by: Joshua Li <[email protected]> Co-authored-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Ashwin P Chandran <[email protected]>

…ql auto suggest (opensearch-project#7543) This PR did: * Added support for MDS (likely Multi-Data Source) in the auto-suggest feature. * Implemented DQL auto-suggest functionality using ANTLR (ANother Tool for Language Recognition). * Updated the query assist feature to use a dataset manager instead of relying on index patterns directly. * Improved field and value suggestions in DQL, including support for partially formed values and phrases. * Added auto-closing quotes and parentheses for DQL in the editor. * Updated the grammar and parser rules for better handling of DQL syntax. * Implemented tests for SQL autocomplete rule processing and DQL field and keyword suggestions. * Made changes to handle the asynchronous nature of the dataset manager in tests. * Cleaned up unnecessary files and updated utilities. --------- Signed-off-by: Kawika Avilla <[email protected]> Signed-off-by: Eric <[email protected]> Signed-off-by: Joshua Li <[email protected]> Signed-off-by: Paul Sebastian <[email protected]> Signed-off-by: Eric Wei <[email protected]> Co-authored-by: Kawika Avilla <[email protected]> Co-authored-by: Joshua Li <[email protected]> Co-authored-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Ashwin P Chandran <[email protected]>

mengweieric requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe, ruanyl, BionIT, xinruiba, zhyuanqi, LDrago27, virajsanghvi and sejli as code owners July 28, 2024 00:56

github-actions bot added the valued-contributor label Jul 28, 2024

github-actions bot added the failed changeset label Jul 28, 2024

github-actions bot removed the failed changeset label Jul 28, 2024

opensearch-changeset-bot bot added a commit to mengweieric/OpenSearch-Dashboards that referenced this pull request Jul 28, 2024

Changeset file for PR opensearch-project#7543 created/updated

14a8e4f

opensearch-changeset-bot bot added a commit to mengweieric/OpenSearch-Dashboards that referenced this pull request Jul 28, 2024

Changeset file for PR opensearch-project#7543 created/updated

3a1c458

add max heigh. use memoization

3efaa55

Signed-off-by: Kawika Avilla <[email protected]> almost working pretty nicely Signed-off-by: Kawika Avilla <[email protected]> a little bit better Signed-off-by: Kawika Avilla <[email protected]> its ok Signed-off-by: Kawika Avilla <[email protected]>

mengweieric force-pushed the feature/add-reverted-sql-ausuggest branch from 3a1c458 to b7fb44b Compare July 29, 2024 19:59

kavilla added 3 commits July 29, 2024 20:50

rebase fixes

aadbe4d

Signed-off-by: Kawika Avilla <[email protected]>

update script

e47af9c

Signed-off-by: Kawika Avilla <[email protected]>

fix initial load

335978d

Signed-off-by: Kawika Avilla <[email protected]>

mengweieric requested a review from joshuali925 as a code owner July 29, 2024 21:27

mengweieric added v2.16.0 backport 2.16 backport 2.x labels Jul 29, 2024

mengweieric and others added 8 commits July 29, 2024 22:56

update snapshot

14841ef

Signed-off-by: Eric <[email protected]>

add back required dependency and minor clean up

d3223af

Signed-off-by: Eric <[email protected]>

minor clean up

70f238c

Signed-off-by: Eric <[email protected]>

support with skip(1)

9a34528

Signed-off-by: Eric <[email protected]>

add back sql auto suggest

e55c76a

Signed-off-by: Eric <[email protected]>

add back disabling of wordBasedSuggestions

7625412

Signed-off-by: Eric <[email protected]>

update tests

f2d381f

Signed-off-by: Eric <[email protected]>

mengweieric force-pushed the feature/add-reverted-sql-ausuggest branch from c43a0ae to f2d381f Compare July 29, 2024 23:03

Merge branch 'main' into feature/add-reverted-sql-ausuggest

9e7b1be

Signed-off-by: Eric Wei <[email protected]>

kavilla reviewed Jul 30, 2024

View reviewed changes

.gitignore Show resolved Hide resolved

kavilla reviewed Jul 30, 2024

View reviewed changes

src/plugins/data/public/ui/query_editor/editors/default_editor/index.tsx Show resolved Hide resolved

ashwin-pc approved these changes Jul 30, 2024

View reviewed changes

joshuali925 approved these changes Jul 30, 2024

View reviewed changes

ananzh removed the backport 2.16 label Jul 30, 2024

ananzh merged commit d0618df into opensearch-project:main Jul 30, 2024
65 of 67 checks passed

opensearch-trigger-bot bot mentioned this pull request Jul 30, 2024

[Backport 2.x] [Auto Suggest] Add back reverted MDS support, tests and cleanup for sql auto suggest #7579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Auto Suggest] Add back reverted MDS support, tests and cleanup for sql auto suggest #7543

[Auto Suggest] Add back reverted MDS support, tests and cleanup for sql auto suggest #7543

mengweieric commented Jul 28, 2024 •

edited

Loading

github-actions bot commented Jul 28, 2024

codecov bot commented Jul 28, 2024 •

edited

Loading

github-actions bot commented Jul 28, 2024

kavilla commented Jul 30, 2024

mengweieric commented Jul 30, 2024

ashwin-pc left a comment

joshuali925 Jul 30, 2024

mengweieric Jul 30, 2024

joshuali925 Jul 30, 2024

joshuali925 Jul 30, 2024

joshuali925 Jul 30, 2024

[Auto Suggest] Add back reverted MDS support, tests and cleanup for sql auto suggest #7543

[Auto Suggest] Add back reverted MDS support, tests and cleanup for sql auto suggest #7543

Conversation

mengweieric commented Jul 28, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Jul 28, 2024

❌ Changelog Entry Missing Hyphen

codecov bot commented Jul 28, 2024 • edited Loading

Codecov Report

github-actions bot commented Jul 28, 2024

❌ Changelog Entry Missing Hyphen

kavilla commented Jul 30, 2024

mengweieric commented Jul 30, 2024

ashwin-pc left a comment

Choose a reason for hiding this comment

joshuali925 Jul 30, 2024

Choose a reason for hiding this comment

mengweieric Jul 30, 2024

Choose a reason for hiding this comment

joshuali925 Jul 30, 2024

Choose a reason for hiding this comment

joshuali925 Jul 30, 2024

Choose a reason for hiding this comment

joshuali925 Jul 30, 2024

Choose a reason for hiding this comment

mengweieric commented Jul 28, 2024 •

edited

Loading

codecov bot commented Jul 28, 2024 •

edited

Loading