fix(structure): respect identifier quote preference

This commit is contained in:
Eve authored and GitHub committed 2026-09-10 13:14:19 +08:00
1 parent dd708fc052
commit e17246c0f6
10 files changed
+77 -13

No files matched your search

@@ -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,
}),
}));
@@ -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<HTMLButtonElement>('[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");
@@ -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) {
@@ -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", () => {
@@ -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,
}),
}));
@@ -19,6 +19,7 @@ vi.mock("@/stores/settingsStore", () => ({
useSettingsStore: () => ({
editorSettings: {
regexMaxMatchCount: 1000,
generateSqlQuoteIdentifiers: true,
},
}),
}));
@@ -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,
}),
}));
@@ -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(),
}),
}));
@@ -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,
}),
}));
@@ -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;
}