diff --git a/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.test.ts b/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.test.ts index f84f70e2df3e..7462c0431180 100644 --- a/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.test.ts +++ b/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.test.ts @@ -44,13 +44,13 @@ const fakeTableApiResult = { result: [ { id: 1, - value: 'fake api result1', + value: 'fake_api_result1', label: 'fake api label1', type: 'table', }, { id: 2, - value: 'fake api result2', + value: 'fake_api_result2', label: 'fake api label2', type: 'table', }, @@ -152,6 +152,64 @@ test('returns keywords including fetched function_names data', async () => { }); }); +test('quotes table identifiers that require quoting in the inserted value', async () => { + const dbFunctionNamesApiRoute = `glob:*/api/v1/database/${expectDbId}/function_names/`; + fetchMock.get(dbFunctionNamesApiRoute, fakeFunctionNamesApiResult); + + act(() => { + store.dispatch( + tableApiUtil.upsertQueryData( + 'tables', + { dbId: expectDbId, schema: expectSchema }, + { + options: [ + { value: 'COVID Vaccines', label: 'COVID Vaccines', type: 'table' }, + { value: 'simple_table', label: 'simple_table', type: 'table' }, + ], + hasMore: false, + }, + ), + ); + }); + + const { result } = renderHook( + () => + useKeywords({ + queryEditorId: 'testqueryid', + dbId: expectDbId, + schema: expectSchema, + }), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + + await waitFor(() => + expect(fetchMock.callHistory.calls(dbFunctionNamesApiRoute).length).toBe(1), + ); + + // A name that needs quoting is inserted as a double-quoted identifier, + // while its display name stays human-readable. + expect(result.current).toContainEqual( + expect.objectContaining({ + name: 'COVID Vaccines', + value: '"COVID Vaccines"', + meta: 'table', + }), + ); + // A simple identifier is inserted as-is, without quotes. + expect(result.current).toContainEqual( + expect.objectContaining({ + name: 'simple_table', + value: 'simple_table', + meta: 'table', + }), + ); +}); + test('skip fetching if autocomplete skipped', () => { const { result } = renderHook( () => diff --git a/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts b/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts index b79a39b27429..afef249a20e4 100644 --- a/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts +++ b/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts @@ -53,6 +53,14 @@ const getHelperText = (value: string) => detail: value, }; +// Names that aren't simple identifiers (spaces, punctuation, leading digits) +// must be double-quoted to be valid SQL, with embedded quotes doubled. +const SIMPLE_IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; +const quoteIdentifier = (identifier: string) => + SIMPLE_IDENTIFIER_RE.test(identifier) + ? identifier + : `"${identifier.replace(/"/g, '""')}"`; + const extensionsRegistry = getExtensionsRegistry(); export function useKeywords( @@ -197,7 +205,7 @@ export function useKeywords( () => allCachedTables.map(({ value, label, schema: tableSchema }) => ({ name: label, - value, + value: quoteIdentifier(value), schema: tableSchema, score: TABLE_AUTOCOMPLETE_SCORE, meta: 'table',