You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/27 00:58:11 UTC

(superset) branch master updated: fix(sqllab): quote autocomplete table names that need it (#41199)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 15626a047c4 fix(sqllab): quote autocomplete table names that need it (#41199)
15626a047c4 is described below

commit 15626a047c4cca2e945470287d48c6c3a416dd21
Author: jesperct <jm...@gmail.com>
AuthorDate: Fri Jun 26 21:58:05 2026 -0300

    fix(sqllab): quote autocomplete table names that need it (#41199)
---
 .../components/EditorWrapper/useKeywords.test.ts   | 62 +++++++++++++++++++++-
 .../SqlLab/components/EditorWrapper/useKeywords.ts | 10 +++-
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.test.ts b/superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.test.ts
index f84f70e2df3..7462c043118 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 b79a39b2742..afef249a20e 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',