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

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,6 @@ snapshots.js

# Yarn local mirror content
.yarn-local-mirror

# Ignore the generated antlr files
/src/plugins/data/public/antlr/opensearch_sql/grammar/.antlr
mengweieric marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions changelogs/fragments/7463.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Add MDS support along with a few cleanup and tests update ([#7463](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7463))
mengweieric marked this conversation as resolved.
Show resolved Hide resolved
70 changes: 25 additions & 45 deletions src/plugins/data/public/antlr/opensearch_sql/code_completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { monaco } from 'packages/osd-monaco/target';
import { monaco } from '@osd/monaco';
import { Lexer as LexerType, ParserRuleContext, Parser as ParserType } from 'antlr4ng';
import { CodeCompletionCore } from 'antlr4-c3';
import {
Expand All @@ -23,7 +23,8 @@
import { openSearchSqlAutocompleteData } from './opensearch_sql_autocomplete';
import { SQL_SYMBOLS } from './constants';
import { QuerySuggestion, QuerySuggestionGetFnArgs } from '../../autocomplete';
import { fetchColumnValues, fetchTableSchemas } from '../shared/utils';
import { fetchTableSchemas } from '../shared/utils';
import { IDataFrameResponse, IFieldType } from '../../../common';

export interface SuggestionParams {
position: monaco.Position;
Expand All @@ -47,80 +48,59 @@
}: QuerySuggestionGetFnArgs): Promise<QuerySuggestion[]> => {
const { api } = services.uiSettings;
const dataSetManager = services.data.query.dataSetManager;
const { lineNumber, column } = position || {};
const suggestions = getOpenSearchSqlAutoCompleteSuggestions(query, {
line: position?.lineNumber || selectionStart,
column: position?.column || selectionEnd,
line: lineNumber || selectionStart,
column: column || selectionEnd,
});

const finalSuggestions = [];
const finalSuggestions: QuerySuggestion[] = [];

Check warning on line 57 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L57 was not covered by tests

try {
// Fetch columns and values
if ('suggestColumns' in suggestions && (suggestions.suggestColumns?.tables?.length ?? 0) > 0) {
const tableNames = suggestions.suggestColumns?.tables?.map((table) => table.name) ?? [];
const schemas = await fetchTableSchemas(tableNames, api, services);

schemas.forEach((schema) => {
if (schema.body?.fields?.length > 0) {
const columns = schema.body.fields.find((col: any) => col.name === 'COLUMN_NAME');
const fieldTypes = schema.body.fields.find((col: any) => col.name === 'DATA_TYPE');
if (suggestions.suggestColumns?.tables?.length) {
const tableNames = suggestions.suggestColumns.tables.map((table) => table.name);
const schemas = await fetchTableSchemas(tableNames, api, dataSetManager);

Check warning on line 63 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/antlr/opensearch_sql/code_completion.ts#L62-L63

Added lines #L62 - L63 were not covered by tests

(schemas as IDataFrameResponse[]).forEach((schema: IDataFrameResponse) => {

Check warning on line 65 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L65 was not covered by tests
mengweieric marked this conversation as resolved.
Show resolved Hide resolved
if ('body' in schema && schema.body && 'fields' in schema.body) {
const columns = schema.body.fields.find((col: IFieldType) => col.name === 'COLUMN_NAME');
const fieldTypes = schema.body.fields.find((col: IFieldType) => col.name === 'DATA_TYPE');

Check warning on line 68 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/data/public/antlr/opensearch_sql/code_completion.ts#L67-L68

Added lines #L67 - L68 were not covered by tests

if (columns && fieldTypes) {
finalSuggestions.push(
...columns.values.map((col: string, index: number) => ({
...columns.values.map((col: string) => ({

Check warning on line 72 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L72 was not covered by tests
text: col,
type: 'field',
fieldType: fieldTypes.values[index],
type: monaco.languages.CompletionItemKind.Field,
}))
);
}
}
});

if (
'suggestValuesForColumn' in suggestions &&
/\S/.test(suggestions.suggestValuesForColumn as string) &&
suggestions.suggestValuesForColumn !== undefined
) {
const values = await fetchColumnValues(
tableNames,
suggestions.suggestValuesForColumn as string,
api,
services
);
values.forEach((value) => {
if (value.body?.fields?.length > 0) {
finalSuggestions.push(
...value.body.fields[0].values.map((colVal: string) => ({
text: `'${colVal}'`,
type: 'value',
}))
);
}
});
}
}

// Fill in aggregate functions
if ('suggestAggregateFunctions' in suggestions && suggestions.suggestAggregateFunctions) {
if (suggestions.suggestAggregateFunctions) {
finalSuggestions.push(
...SQL_SYMBOLS.AGREGATE_FUNCTIONS.map((af) => ({
text: af,
type: 'function',
type: monaco.languages.CompletionItemKind.Function,
}))
);
}

// Fill in SQL keywords
if ('suggestKeywords' in suggestions && (suggestions.suggestKeywords?.length ?? 0) > 0) {
if (suggestions.suggestKeywords?.length) {
finalSuggestions.push(
...(suggestions.suggestKeywords ?? []).map((sk) => ({
...suggestions.suggestKeywords.map((sk) => ({

Check warning on line 95 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L95 was not covered by tests
text: sk.value,
type: 'keyword',
type: monaco.languages.CompletionItemKind.Keyword,
}))
);
}
} catch (error) {
// TODO: pipe error to the UI
// TODO: Handle errors appropriately, possibly logging or displaying a message to the user
return [];

Check warning on line 103 in src/plugins/data/public/antlr/opensearch_sql/code_completion.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L103 was not covered by tests
Copy link
Member

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?

Copy link
Collaborator Author

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?

Copy link
Member

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

}

return finalSuggestions;
Expand Down
Loading
Loading