-
Notifications
You must be signed in to change notification settings - Fork 884
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
[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
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
Codecov ReportAttention: Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
Signed-off-by: Kawika Avilla <kavilla414@gmail.com> almost working pretty nicely Signed-off-by: Kawika Avilla <kavilla414@gmail.com> a little bit better Signed-off-by: Kawika Avilla <kavilla414@gmail.com> its ok Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
3a1c458
to
b7fb44b
Compare
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
…ensearch-project#7463) * add tests for sql autocomplete rule processing Signed-off-by: Eric <menwe@amazon.com> * refer to monaco type directly Signed-off-by: Eric <menwe@amazon.com> * remove unnecessary antlr auto generated files Signed-off-by: Eric <menwe@amazon.com> * inital adoption of dataSet manager Signed-off-by: Eric <menwe@amazon.com> * mds support Signed-off-by: Eric <menwe@amazon.com> * remove test that are failed due to adopting dataSet manager Signed-off-by: Eric <menwe@amazon.com> * add changelog Signed-off-by: Eric <menwe@amazon.com> * 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 <joshuali925@gmail.com> * add dataset mock to query mock Signed-off-by: Joshua Li <joshuali925@gmail.com> * update query assist to use dataset manager Signed-off-by: Joshua Li <joshuali925@gmail.com> * use selected dataset state instead of relying on rerender Signed-off-by: Joshua Li <joshuali925@gmail.com> * remove skip 1 in dataset observable Signed-off-by: Joshua Li <joshuali925@gmail.com> * update dataset_manager tests Signed-off-by: Joshua Li <joshuali925@gmail.com> --------- Signed-off-by: Joshua Li <joshuali925@gmail.com> * update utils Signed-off-by: Eric <menwe@amazon.com> * keep with observable and remove values suggestion Signed-off-by: Eric <menwe@amazon.com> * update unit tests Signed-off-by: Eric <menwe@amazon.com> * [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 <paulstn@amazon.com> * show suggestion of fields depending on current index pattern Signed-off-by: Paul Sebastian <paulstn@amazon.com> * basic code completion with fields populated Signed-off-by: Paul Sebastian <paulstn@amazon.com> * updated grammar and generated for better group handling Signed-off-by: Paul Sebastian <paulstn@amazon.com> * add ignored tokens Signed-off-by: Paul Sebastian <paulstn@amazon.com> * remove console logs Signed-off-by: Paul Sebastian <paulstn@amazon.com> --------- Signed-off-by: Paul Sebastian <paulstn@amazon.com> * dql Antlr autocomplete (opensearch-project#7160) * re-add provider for sql Signed-off-by: Paul Sebastian <paulstn@amazon.com> * added temporary fix for language providor to appear for more than one language Signed-off-by: Paul Sebastian <paulstn@amazon.com> --------- Signed-off-by: Paul Sebastian <paulstn@amazon.com> * remove EOF in parser to fix suggestions Signed-off-by: Paul Sebastian <paulstn@amazon.com> * use custom version of cursor token index for dql Signed-off-by: Paul Sebastian <paulstn@amazon.com> * implemented value suggestions based on field Signed-off-by: Paul Sebastian <paulstn@amazon.com> * set param type Signed-off-by: Paul Sebastian <paulstn@amazon.com> * update grouping grammar Signed-off-by: Paul Sebastian <paulstn@amazon.com> * fix grammar for dots in field and value term search with spaces Signed-off-by: Paul Sebastian <paulstn@amazon.com> * value suggestions match field to avoid failing api call and to find assc keyword field Signed-off-by: Paul Sebastian <paulstn@amazon.com> * update value suggestions from partially formed value Signed-off-by: Paul Sebastian <paulstn@amazon.com> * refactor value suggestions and change fieldval listener to visitor Signed-off-by: Paul Sebastian <paulstn@amazon.com> * implement value suggestions within phrases Signed-off-by: Paul Sebastian <paulstn@amazon.com> * make grammar more readable Signed-off-by: Paul Sebastian <paulstn@amazon.com> * rename grammar parser rules Signed-off-by: Paul Sebastian <paulstn@amazon.com> * bring back minimal autocomplete optimized grammar Signed-off-by: Paul Sebastian <paulstn@amazon.com> * enable partially complete value suggestion for value groups Signed-off-by: Paul Sebastian <paulstn@amazon.com> * remove number as lexer rule Signed-off-by: Paul Sebastian <paulstn@amazon.com> * fix cursor import and clean up Signed-off-by: Paul Sebastian <paulstn@amazon.com> * fix completion item range to be current word Signed-off-by: Paul Sebastian <paulstn@amazon.com> * update cursor to use monaco position Signed-off-by: Paul Sebastian <paulstn@amazon.com> * cursor index to use position directly Signed-off-by: Paul Sebastian <paulstn@amazon.com> * move language registration into render function to handle new languages Signed-off-by: Paul Sebastian <paulstn@amazon.com> * include auto closing quotes and parenthesis for dql Signed-off-by: Paul Sebastian <paulstn@amazon.com> * rename generated file Signed-off-by: Paul Sebastian <paulstn@amazon.com> * include single line editor closing pairs Signed-off-by: Paul Sebastian <paulstn@amazon.com> * Changeset file for PR opensearch-project#7391 created/updated * add license and fix linting Signed-off-by: Paul Sebastian <paulstn@amazon.com> * modify grammar Signed-off-by: Paul Sebastian <paulstn@amazon.com> * add tests for fields and keywords Signed-off-by: Paul Sebastian <paulstn@amazon.com> * move dql test constants to separate file Signed-off-by: Paul Sebastian <paulstn@amazon.com> * pass core setup from autocomplete constructor to query sugg provider and utilize selectionEnd if no position Signed-off-by: Paul Sebastian <paulstn@amazon.com> * update an import Signed-off-by: Paul Sebastian <paulstn@amazon.com> * use updated dataset for index pattern Signed-off-by: Paul Sebastian <paulstn@amazon.com> * remove console log Signed-off-by: Paul Sebastian <paulstn@amazon.com> --------- Signed-off-by: Paul Sebastian <paulstn@amazon.com> 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 <kavilla414@gmail.com> * bad fingers accidentally hit the x button Signed-off-by: Kawika Avilla <kavilla414@gmail.com> --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com> * resolve conflicts Signed-off-by: Eric <menwe@amazon.com> * fix one minor linting Signed-off-by: Eric <menwe@amazon.com> --------- Signed-off-by: Eric <menwe@amazon.com> Signed-off-by: Joshua Li <joshuali925@gmail.com> Signed-off-by: Paul Sebastian <paulstn@amazon.com> Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: Eric Wei <menwe@amazon.com> Co-authored-by: Joshua Li <joshuali925@gmail.com> Co-authored-by: Paul Sebastian <paulstn@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
Signed-off-by: Eric <menwe@amazon.com>
c43a0ae
to
f2d381f
Compare
Signed-off-by: Eric Wei <menwe@amazon.com>
src/plugins/data/public/ui/query_editor/editors/default_editor/index.tsx
Show resolved
Hide resolved
Some questions that if you have some insight could unblock/give peace of mind.
|
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. |
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.
Approving given the response.
})) | ||
); | ||
} | ||
} catch (error) { | ||
// TODO: pipe error to the UI | ||
// TODO: Handle errors appropriately, possibly logging or displaying a message to the user | ||
return []; |
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.
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?
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.
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?
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 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
}) | ||
); | ||
|
||
const fetchFromAPI = async (api: any, body: string) => { | ||
try { | ||
return await api.http.fetch({ |
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.
what is api
and why not just return api.http.fetch without try catch?
} | ||
).subscribe({ | ||
next: (dataFrames: any) => resolve(dataFrames), | ||
error: (err: any) => { | ||
error: (err: Error) => { |
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.
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 <kavilla414@gmail.com> Signed-off-by: Eric <menwe@amazon.com> Signed-off-by: Joshua Li <joshuali925@gmail.com> Signed-off-by: Paul Sebastian <paulstn@amazon.com> Signed-off-by: Eric Wei <menwe@amazon.com> Co-authored-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: Joshua Li <joshuali925@gmail.com> Co-authored-by: Paul Sebastian <paulstn@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com> (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 <kavilla414@gmail.com> Signed-off-by: Eric <menwe@amazon.com> Signed-off-by: Joshua Li <joshuali925@gmail.com> Signed-off-by: Paul Sebastian <paulstn@amazon.com> Signed-off-by: Eric Wei <menwe@amazon.com> 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 <kavilla414@gmail.com> Co-authored-by: Joshua Li <joshuali925@gmail.com> Co-authored-by: Paul Sebastian <paulstn@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>
…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 <kavilla414@gmail.com> Signed-off-by: Eric <menwe@amazon.com> Signed-off-by: Joshua Li <joshuali925@gmail.com> Signed-off-by: Paul Sebastian <paulstn@amazon.com> Signed-off-by: Eric Wei <menwe@amazon.com> Co-authored-by: Kawika Avilla <kavilla414@gmail.com> Co-authored-by: Joshua Li <joshuali925@gmail.com> Co-authored-by: Paul Sebastian <paulstn@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>
Description
Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration