fix(sqllab): key overwrite-dataset options by id, not table name

superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx

@@ -30,6 +30,7 @@ import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
 import { createDatasource } from 'src/SqlLab/actions/sqlLab';
 import { user, testQuery, mockdatasets } from 'src/SqlLab/fixtures';
 import { FeatureFlag, SupersetClient } from '@superset-ui/core';
+import rison from 'rison';
 
 const mockedProps = {
   visible: true,
@@ -193,7 +194,7 @@ describe('SaveDatasetModal', () => {
     });
 
     // Select the first "existing dataset" from the listbox
-    const option = screen.getAllByText('coolest table 0')[1];
+    const option = screen.getAllByText('schema 0.coolest table 0')[0];
     await userEvent.click(option);
 
     // Overwrite button should now be enabled
@@ -224,7 +225,7 @@ describe('SaveDatasetModal', () => {
     });
 
     // Select the first "existing dataset" from the listbox
-    const option = screen.getAllByText('coolest table 0')[1];
+    const option = screen.getAllByText('schema 0.coolest table 0')[0];
     await userEvent.click(option);
 
     // Click the overwrite button to access the confirmation screen
@@ -246,6 +247,137 @@ describe('SaveDatasetModal', () => {
     ).toBeInTheDocument();
   });
 
+  test('distinguishes datasets that share a table name and overwrites the selected one', async () => {
+    // Datasets are unique by database, catalog, schema and table name, so the
+    // same table name can legitimately appear several times.
+    const sameNameDatasets = [
+      {
+        ...mockdatasets[0],
+        id: 11,
+        database: { database_name: 'warehouse' },
+        schema: 'staging',
+        table_name: 'task_instance',
+      },
+      {
+        ...mockdatasets[0],
+        id: 22,
+        database: { database_name: 'warehouse' },
+        schema: 'prod',
+        table_name: 'task_instance',
+      },
+      // Same schema and table as the one above — only the database differs
+      {
+        ...mockdatasets[0],
+        id: 33,
+        database: { database_name: 'analytics' },
+        catalog: 'reporting',
+        schema: 'prod',
+        table_name: 'task_instance',
+      },
+    ];
+    // Pad past a single API page so the search cannot be served from options
+    // already in memory — it has to reach the API.
+    const PAGE_SIZE = 100;
+    const allDatasets = [
+      ...sameNameDatasets,
+      ...Array.from({ length: PAGE_SIZE * 2 }, (_, i) => ({
+        ...mockdatasets[0],
+        id: 1000 + i,
+        database: { database_name: 'warehouse' },
+        schema: `schema_${i}`,
+        table_name: `table_${i}`,
+      })),
+    ];
+    // Filter and paginate the way the API does — a mock that returns
+    // everything regardless of the search hides server-side mismatches.
+    const getSpy = jest
+      .spyOn(SupersetClient, 'get')
+      .mockImplementation(({ endpoint }: any) => {
+        const { filters } = rison.decode(
+          endpoint.slice(endpoint.indexOf('q=') + 2),
+        ) as { filters: { col: string; opr: string; value: any }[] };
+        const matches = allDatasets.filter(dataset =>
+          filters.every(({ col, value }) =>
+            col === 'table_name' ? dataset.table_name.includes(value) : true,
+          ),
+        );
+        return Promise.resolve({
+          json: { result: matches.slice(0, PAGE_SIZE), count: matches.length },
+        }) as any;
+      });
+    const putSpy = jest
+      .spyOn(SupersetClient, 'put')
+      .mockResolvedValue({ json: { result: { id: 33 } } } as any);
+
+    renderModal();
+
+    await userEvent.click(
+      screen.getByRole('radio', { name: /overwrite existing/i }),
+    );
+    const combobox = screen.getByRole('combobox', {
+      name: /existing dataset/i,
+    });
+    await userEvent.click(combobox);
+    await act(async () => {
+      jest.runAllTimers();
+    });
+    await waitFor(() => {
+      const loading = screen.queryByText('Loading...');
+      expect(loading === null || !loading.checkVisibility()).toBe(true);
+    });
+
+    await userEvent.type(combobox, 'task_instance');
+    await act(async () => {
+      jest.runAllTimers();
+    });
+    expect(
+      await screen.findAllByText('warehouse.staging.task_instance'),
+    ).toHaveLength(1);
+    expect(
+      await screen.findAllByText('warehouse.prod.task_instance'),
+    ).toHaveLength(1);
+    expect(
+      await screen.findAllByText('analytics.reporting.prod.task_instance'),
+    ).toHaveLength(1);
+
+    // Qualifying the search narrows the list, skipping the catalog included.
+    await userEvent.clear(combobox);
+    await userEvent.click(combobox);
+    await userEvent.type(combobox, 'analytics.prod.task_instance');
+    await act(async () => {
+      jest.runAllTimers();
+    });
+    await act(async () => {
+      jest.runAllTimers();
+    });
+    await waitFor(() =>
+      expect(
+        screen.queryByText('warehouse.prod.task_instance'),
+      ).not.toBeInTheDocument(),
+    );
+    expect(
+      screen.queryByText('warehouse.staging.task_instance'),
+    ).not.toBeInTheDocument();
+
+    await userEvent.click(
+      screen.getByText('analytics.reporting.prod.task_instance'),
+    );
+    await userEvent.click(screen.getByRole('button', { name: /overwrite/i }));
+    await screen.findByText(/are you sure you want to overwrite this dataset/i);
+    await userEvent.click(screen.getByRole('button', { name: /overwrite/i }));
+
+    await waitFor(() => {
+      expect(
+        putSpy.mock.calls.some(([req]) =>
+          req.endpoint?.includes('api/v1/dataset/33'),
+        ),
+      ).toBe(true);
+    });
+
+    getSpy.mockRestore();
+    putSpy.mockRestore();
+  });
+
   test('sends the schema when creating the dataset', async () => {
     renderModal();
 
@@ -401,8 +533,8 @@ describe('SaveDatasetModal', () => {
       expect(loading === null || !loading.checkVisibility()).toBe(true);
     });
     // Pick an existing dataset (use the listbox item, not the input mirror)
-    const options = await screen.findAllByText('coolest table 0');
-    await userEvent.click(options[1]);
+    const options = await screen.findAllByText('schema 0.coolest table 0');
+    await userEvent.click(options[0]);
     // First overwrite click → confirmation screen
     await userEvent.click(screen.getByRole('button', { name: /overwrite/i }));
     // Wait for the confirmation screen to render

superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx

@@ -195,6 +195,37 @@ const updateDataset = async ({
 
 const UNTITLED = t('Untitled Dataset');
 
+/**
+ * Datasets are unique by database, catalog, schema and table name, so a label
+ * built from anything less can be ambiguous — e.g. `examples.public.sales`.
+ */
+const qualifiedLabel = (dataset: {
+  database?: { database_name?: string };
+  catalog?: string | null;
+  schema?: string | null;
+  table_name: string;
+}) =>
+  [
+    dataset.database?.database_name,
+    dataset.catalog,
+    dataset.schema,
+    dataset.table_name,
+  ]
+    .filter(Boolean)
+    .join('.');
+
+/**
+ * Break a search typed against those labels into its parts. The trailing part
+ * is the table name, unless the user has just typed a separator.
+ */
+const parseQualifiedSearch = (input: string) => {
+  const parts = input.split('.').filter(Boolean);
+  return {
+    parts,
+    tableSearch: input.endsWith('.') ? '' : (parts[parts.length - 1] ?? ''),
+  };
+};
+
 // The filters param is only used to test jinja templates.
 // Remove the special filters entry from the templateParams
 // before saving the dataset.
@@ -321,12 +352,18 @@ export const SaveDatasetModal = ({
   };
 
   const loadDatasetOverwriteOptions = useCallback(async (input = '') => {
+    // Only the table part can be filtered server-side — `database` is a
+    // relationship the list endpoint cannot match on by name. Sending the
+    // whole search as a `table_name` filter matches nothing once the user
+    // types a separator; filterAutocompleteOption narrows the qualifiers.
+    const { tableSearch } = parseQualifiedSearch(input);
+
     const queryParams = rison.encode({
       filters: [
         {
           col: 'table_name',
           opr: 'ct',
-          value: input,
+          value: tableSearch,
         },
         {
           col: 'id',
@@ -342,9 +379,18 @@ export const SaveDatasetModal = ({
       endpoint: `/api/v1/dataset/?q=${queryParams}`,
     }).then(response => ({
       data: response.json.result.map(
-        (r: { table_name: string; id: number; editors: Subject[] }) => ({
-          value: r.table_name,
-          label: r.table_name,
+        (r: {
+          table_name: string;
+          id: number;
+          editors: Subject[];
+          database?: { database_name?: string };
+          catalog?: string | null;
+          schema?: string | null;
+        }) => ({
+          // `id` is unique; `table_name` is not. Keying by the table name
+          // collapses same-named datasets onto a single Select key.
+          value: r.id,
+          label: qualifiedLabel(r),
           datasetId: r.id,
           editors: r.editors,
         }),
@@ -424,7 +470,14 @@ export const SaveDatasetModal = ({
   const filterAutocompleteOption = (
     inputValue: string,
     option: DatasetOverwriteOption,
-  ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+  ) => {
+    const label = option.label.toLowerCase();
+    // Position-independent: a dataset may or may not have a catalog, and a
+    // search skipping a part (`examples.sales`) should still match.
+    return parseQualifiedSearch(inputValue.toLowerCase()).parts.every(part =>
+      label.includes(part),
+    );
+  };
 
   return (
     <Modal

superset-frontend/src/SqlLab/types.ts

@@ -137,7 +137,7 @@ export const EXPLORE_CHART_DEFAULT = {
 };
 
 export interface DatasetOptionAutocomplete {
-  value: string;
+  value: number;
   datasetId: number;
   editors: Subject[];
 }