fix(sqllab): quote autocomplete table names that need it

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(
     () =>

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',