From e17246c0f68c4c7f85a5e028a3a82113ab2930da Mon Sep 17 00:00:00 2001 From: Eve <92513238+Eve-z1@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:14:19 +0800 Subject: [PATCH] fix(structure): respect identifier quote preference --- ...leStructureEditor.charsetCollation.spec.ts | 2 +- .../TableStructureEditor.primaryKey.spec.ts | 38 ++++++++++++++++++- .../structure/TableStructureEditor.vue | 5 ++- .../TableStructureEditor.concurrent.spec.ts | 21 ++++++++++ ...ructureEditor.concurrentTransition.spec.ts | 2 +- .../TableStructureEditor.ddlSearch.spec.ts | 1 + .../TableStructureEditor.ddlTab.spec.ts | 2 +- ...bleStructureEditor.ddlTabLifecycle.spec.ts | 2 +- .../TableStructureEditor.initialTab.spec.ts | 2 +- .../lib/table/concurrentIndexAvailability.ts | 15 +++++--- 10 files changed, 77 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/components/structure/TableStructureEditor.charsetCollation.spec.ts b/apps/desktop/src/components/structure/TableStructureEditor.charsetCollation.spec.ts index a263443d9f..2a235cf1d4 100644 --- a/apps/desktop/src/components/structure/TableStructureEditor.charsetCollation.spec.ts +++ b/apps/desktop/src/components/structure/TableStructureEditor.charsetCollation.spec.ts @@ -211,7 +211,7 @@ vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRe vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ - editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [] }, + editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], generateSqlQuoteIdentifiers: true }, updateEditorSettings: mocks.updateEditorSettings, }), })); diff --git a/apps/desktop/src/components/structure/TableStructureEditor.primaryKey.spec.ts b/apps/desktop/src/components/structure/TableStructureEditor.primaryKey.spec.ts index 0c30a285d9..24624f60a6 100644 --- a/apps/desktop/src/components/structure/TableStructureEditor.primaryKey.spec.ts +++ b/apps/desktop/src/components/structure/TableStructureEditor.primaryKey.spec.ts @@ -14,6 +14,7 @@ const mocks = vi.hoisted(() => ({ executeQuery: vi.fn(), listDataTypes: vi.fn(), buildTableStructureChangeSql: vi.fn(), + previewSqliteTableStructureChange: vi.fn(), buildMysqlAutoIncrementSql: vi.fn(), updateEditorSettings: vi.fn(), loadObjectDdl: vi.fn(), @@ -24,6 +25,12 @@ const mocks = vi.hoisted(() => ({ getTablePartitionStatus: vi.fn(), getTableOwner: vi.fn(), buildTableOwnerChangeSql: vi.fn(), + editorSettings: { + structureEditorDensity: "compact", + sqlFormatter: {}, + tableColumnTemplateFields: [], + generateSqlQuoteIdentifiers: true, + }, toast: vi.fn(), })); @@ -212,7 +219,7 @@ vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRe vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ - editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [] }, + editorSettings: mocks.editorSettings, updateEditorSettings: mocks.updateEditorSettings, }), })); @@ -229,6 +236,7 @@ vi.mock("@/lib/backend/api", () => ({ executeQuery: mocks.executeQuery, listDataTypes: mocks.listDataTypes, buildTableStructureChangeSql: mocks.buildTableStructureChangeSql, + previewSqliteTableStructureChange: mocks.previewSqliteTableStructureChange, buildMysqlAutoIncrementSql: mocks.buildMysqlAutoIncrementSql, buildTableOwnerChangeSql: mocks.buildTableOwnerChangeSql, getTablePartitionStatus: mocks.getTablePartitionStatus, @@ -352,6 +360,8 @@ function buttonWithText(root: HTMLElement, text: string): HTMLButtonElement { beforeEach(() => { vi.clearAllMocks(); + mocks.editorSettings.generateSqlQuoteIdentifiers = true; + mocks.previewSqliteTableStructureChange.mockResolvedValue({ statements: [], warnings: [], schemaRevision: "sqlite-revision" }); mocks.loadObjectDdl.mockResolvedValue({ ddl: "CREATE TABLE users (id bigint)", cacheStatus: "remote" }); mocks.invalidateObjectDdl.mockResolvedValue(undefined); mocks.invalidateObjectMetadataCache.mockResolvedValue(undefined); @@ -572,6 +582,32 @@ describe("TableStructureEditor primary key editing", () => { expect(buttonWithText(root, "structureEditor.apply").disabled).toBe(false); }); + it.each([ + ["postgres", "public"], + ["sqlite", "main"], + ] as const)("omits safe identifier quotes from generated %s SQL when quoting is disabled", async (databaseType, schema) => { + mocks.editorSettings.generateSqlQuoteIdentifiers = false; + const root = await mountEditor(databaseType); + mocks.buildTableStructureChangeSql.mockResolvedValueOnce({ statements: [`ALTER TABLE "${schema}"."demo_table" ADD "abc" VARCHAR(20) DEFAULT 'quoted value'`], warnings: [] }); + + buttonWithText(root, "structureEditor.addColumn").click(); + + await vi.waitFor(() => expect(root.textContent).toContain(`ALTER TABLE ${schema}.demo_table ADD abc VARCHAR(20) DEFAULT 'quoted value'`)); + expect(root.textContent).not.toContain(`"${schema}"."demo_table"`); + }); + + it("keeps SQLite rebuild SQL aligned with the guarded apply plan", async () => { + mocks.editorSettings.generateSqlQuoteIdentifiers = false; + const root = await mountEditor("sqlite"); + const preview = `ALTER TABLE "main"."demo_table" RENAME COLUMN "old_name" TO "new_name"`; + mocks.previewSqliteTableStructureChange.mockResolvedValueOnce({ statements: [preview], warnings: [], schemaRevision: "sqlite-revision" }); + + root.querySelector('[data-searchable-select="true"]')?.click(); + + await vi.waitFor(() => expect(mocks.previewSqliteTableStructureChange).toHaveBeenCalled()); + expect(root.textContent).toContain(preview); + }); + it("debounces SQL preview generation while editing a column name", async () => { vi.useFakeTimers(); const root = await mountEditor("dameng"); diff --git a/apps/desktop/src/components/structure/TableStructureEditor.vue b/apps/desktop/src/components/structure/TableStructureEditor.vue index 9d79146405..943e9620bd 100644 --- a/apps/desktop/src/components/structure/TableStructureEditor.vue +++ b/apps/desktop/src/components/structure/TableStructureEditor.vue @@ -27,6 +27,7 @@ import { createDbxCodeMirrorSqlDialect } from "@/lib/editor/codemirrorSqlDialect import { useToast } from "@/composables/useToast"; import { type SqlHighlighter, createShikiSqlHighlighter } from "@/lib/sql/sqlHighlighter"; import { joinSqlStatementsForScript } from "@/lib/sql/sqlBatchScript"; +import { omitDdlIdentifierQuotes } from "@/lib/sql/ddlDisplay"; import { splitSqlStatementRanges } from "@/lib/sql/sqlStatementRanges"; import { copyToClipboard } from "@/lib/common/clipboard"; import { formatSqlForDisplay, sqlFormatDialectForDbType } from "@/lib/sql/sqlFormatter"; @@ -1717,7 +1718,9 @@ async function refreshSqlPreview() { }), ]); if (requestId !== sqlPreviewRequestId) return; - pendingStatements.value = [...result.statements, ...ownerResult.statements, ...(mysqlAutoIncrementStatement ? [mysqlAutoIncrementStatement] : [])]; + const statements = [...result.statements, ...ownerResult.statements, ...(mysqlAutoIncrementStatement ? [mysqlAutoIncrementStatement] : [])]; + // SQLite type-change apply regenerates this revision-checked plan, so its preview must stay byte-for-byte aligned. + pendingStatements.value = settingsStore.editorSettings.generateSqlQuoteIdentifiers || hasSqliteTypeChange.value ? statements : statements.map((statement) => omitDdlIdentifierQuotes(statement, sqlFormatDialectForDbType(databaseType.value))); warnings.value = [...result.warnings, ...ownerResult.warnings]; sqliteSchemaRevision.value = "schemaRevision" in result && typeof result.schemaRevision === "string" ? result.schemaRevision : undefined; } catch (e: any) { diff --git a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrent.spec.ts b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrent.spec.ts index 40d87945b2..5f82c308f0 100644 --- a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrent.spec.ts +++ b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrent.spec.ts @@ -76,6 +76,27 @@ describe("concurrentIndexNamesInStatements", () => { expect(concurrentIndexNamesInStatements([])).toEqual([]); expect(concurrentIndexNamesInStatements(['CREATE INDEX "idx" ON "public"."t" ("c");'])).toEqual([]); }); + + it("extracts names from dequoted statements when the identifier-quote preference is off", () => { + expect( + concurrentIndexNamesInStatements([ + "CREATE INDEX CONCURRENTLY idx_users_email ON public.users (email);", + "CREATE UNIQUE INDEX CONCURRENTLY uniq_users_name ON public.users (name);", + 'CREATE INDEX CONCURRENTLY "MixedCase".idx_orders_id ON orders.orders (id);', + "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ifne ON public.users (id);", + ]), + ).toEqual(["idx_users_email", "uniq_users_name", "idx_orders_id", "idx_ifne"]); + }); + + it("still detects a same-name INVALID index after dequoting (quote preference off)", () => { + // Regression for the pre-apply guard at TableStructureEditor applyChanges: + // with generateSqlQuoteIdentifiers disabled, pendingStatements are dequoted + // before the guard runs, so the extracted name must still match the + // listInvalidIndexes result and block the save. + const invalidIndexes = ["idx_users_email"]; + const concurrentIndexNames = concurrentIndexNamesInStatements(["CREATE INDEX CONCURRENTLY idx_users_email ON public.users (email);"]); + expect(concurrentIndexNames.filter((name) => invalidIndexes.includes(name))).toEqual(["idx_users_email"]); + }); }); describe("normalizeUnsupportedConcurrentIndexes", () => { diff --git a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrentTransition.spec.ts b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrentTransition.spec.ts index 58f6be0a39..21d5950053 100644 --- a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrentTransition.spec.ts +++ b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.concurrentTransition.spec.ts @@ -227,7 +227,7 @@ vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRe vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ - editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [] }, + editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], generateSqlQuoteIdentifiers: true }, updateEditorSettings: mocks.updateEditorSettings, }), })); diff --git a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlSearch.spec.ts b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlSearch.spec.ts index 2e8e561fe7..4797e7bb03 100644 --- a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlSearch.spec.ts +++ b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlSearch.spec.ts @@ -19,6 +19,7 @@ vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ editorSettings: { regexMaxMatchCount: 1000, + generateSqlQuoteIdentifiers: true, }, }), })); diff --git a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTab.spec.ts b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTab.spec.ts index 4bcd86c8fa..672a50e3b4 100644 --- a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTab.spec.ts +++ b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTab.spec.ts @@ -259,7 +259,7 @@ vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRe vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ - editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], fontSize: 13, fontFamily: "monospace", theme: "default" }, + editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], fontSize: 13, fontFamily: "monospace", theme: "default", generateSqlQuoteIdentifiers: true }, updateEditorSettings: mocks.updateEditorSettings, }), })); diff --git a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTabLifecycle.spec.ts b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTabLifecycle.spec.ts index 01b4e6042b..583db6d818 100644 --- a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTabLifecycle.spec.ts +++ b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.ddlTabLifecycle.spec.ts @@ -176,7 +176,7 @@ vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRe vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ - editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], theme: "default", fontSize: 13, fontFamily: "monospace" }, + editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], theme: "default", fontSize: 13, fontFamily: "monospace", generateSqlQuoteIdentifiers: true }, updateEditorSettings: vi.fn(), }), })); diff --git a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.initialTab.spec.ts b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.initialTab.spec.ts index a6d8e74528..bf4c0ff079 100644 --- a/apps/desktop/src/components/structure/__tests__/TableStructureEditor.initialTab.spec.ts +++ b/apps/desktop/src/components/structure/__tests__/TableStructureEditor.initialTab.spec.ts @@ -259,7 +259,7 @@ vi.mock("@/stores/queryStore", () => ({ useQueryStore: () => ({ tableStructureRe vi.mock("@/stores/historyStore", () => ({ useHistoryStore: () => ({ add: vi.fn() }) })); vi.mock("@/stores/settingsStore", () => ({ useSettingsStore: () => ({ - editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], fontSize: 13, fontFamily: "monospace", theme: "default" }, + editorSettings: { structureEditorDensity: "compact", sqlFormatter: {}, tableColumnTemplateFields: [], fontSize: 13, fontFamily: "monospace", theme: "default", generateSqlQuoteIdentifiers: true }, updateEditorSettings: mocks.updateEditorSettings, }), })); diff --git a/apps/desktop/src/lib/table/concurrentIndexAvailability.ts b/apps/desktop/src/lib/table/concurrentIndexAvailability.ts index d7703b33cf..e5d90891fd 100644 --- a/apps/desktop/src/lib/table/concurrentIndexAvailability.ts +++ b/apps/desktop/src/lib/table/concurrentIndexAvailability.ts @@ -43,16 +43,19 @@ export function getConcurrentIndexAvailability(input: ConcurrentIndexAvailabilit } /** - * Unquoted index names referenced by `CREATE [UNIQUE] INDEX CONCURRENTLY` - * statements, used to detect same-name INVALID leftovers before applying a - * concurrent build. The PG builder emits the index name as a single - * double-quoted identifier (optionally schema-qualified). + * Index names referenced by `CREATE [UNIQUE] INDEX CONCURRENTLY` statements, + * used to detect same-name INVALID leftovers before applying a concurrent + * build. The PG builder emits the index name as a double-quoted identifier + * (optionally schema-qualified); when the identifier-quote preference is off, + * the structure editor dequotes safe names before these statements reach this + * check, so unquoted names are accepted as well. */ export function concurrentIndexNamesInStatements(statements: string[]): string[] { const names: string[] = []; for (const sql of statements) { - const match = sql.match(/CREATE\s+(?:UNIQUE\s+)?INDEX\s+CONCURRENTLY\s+(?:(?:"[^"]+")\s*\.\s*)?"([^"]+)"/i); - if (match?.[1]) names.push(match[1]); + const match = sql.match(/CREATE\s+(?:UNIQUE\s+)?INDEX\s+CONCURRENTLY\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:(?:"[^"]+"|[A-Za-z_][A-Za-z0-9_$]*)\s*\.\s*)?(?:"([^"]+)"|([A-Za-z_][A-Za-z0-9_$]*))/i); + const name = match?.[1] ?? match?.[2]; + if (name) names.push(name); } return names; }