diff --git a/apps/desktop/src/components/structure/TableStructureEditorDialog.vue b/apps/desktop/src/components/structure/TableStructureEditorDialog.vue index f128c08e1f..40f2325e85 100644 --- a/apps/desktop/src/components/structure/TableStructureEditorDialog.vue +++ b/apps/desktop/src/components/structure/TableStructureEditorDialog.vue @@ -308,13 +308,23 @@ function toggleDropIndex(index: EditableStructureIndex) { } function canEditIndexDraft(index: EditableStructureIndex): boolean { - return !index.original && !index.markedForDrop && structureCapabilities.value.createIndex; + if (index.markedForDrop || index.isPrimary) return false; + if (!index.original) return structureCapabilities.value.createIndex; + return ( + structureCapabilities.value.rebuildIndex && + structureCapabilities.value.createIndex && + structureCapabilities.value.dropIndex + ); } function canEditIndexFilter(index: EditableStructureIndex): boolean { return canEditIndexDraft(index) && structureCapabilities.value.indexFilter; } +function canEditIndexComment(index: EditableStructureIndex): boolean { + return canEditIndexDraft(index) && structureCapabilities.value.indexComment; +} + function canDropIndex(index: EditableStructureIndex): boolean { return !!index.original && !index.isPrimary && structureCapabilities.value.dropIndex; } @@ -640,11 +650,8 @@ watch( - {{ - index.indexType || "BTREE" - }} + {{ t("structureEditor.primary") }} diff --git a/apps/desktop/src/lib/tableStructureCapabilities.ts b/apps/desktop/src/lib/tableStructureCapabilities.ts index 22fbec3a06..c29423c0bd 100644 --- a/apps/desktop/src/lib/tableStructureCapabilities.ts +++ b/apps/desktop/src/lib/tableStructureCapabilities.ts @@ -24,6 +24,7 @@ export interface TableStructureCapabilities { comment: boolean; createIndex: boolean; dropIndex: boolean; + rebuildIndex: boolean; indexType: boolean; indexInclude: boolean; indexFilter: boolean; @@ -43,6 +44,7 @@ const unsupportedCapabilities: TableStructureCapabilities = { comment: false, createIndex: false, dropIndex: false, + rebuildIndex: false, indexType: false, indexInclude: false, indexFilter: false, @@ -66,6 +68,7 @@ const mysqlCapabilities = capabilities({ comment: true, createIndex: true, dropIndex: true, + rebuildIndex: true, indexType: true, }); @@ -82,6 +85,7 @@ const postgresCapabilities = capabilities({ comment: true, createIndex: true, dropIndex: true, + rebuildIndex: true, indexType: true, indexInclude: true, indexFilter: true, @@ -92,6 +96,7 @@ const redshiftCapabilities = capabilities({ ...postgresCapabilities, createIndex: false, dropIndex: false, + rebuildIndex: false, indexType: false, indexInclude: false, indexFilter: false, @@ -106,6 +111,7 @@ const sqliteCapabilities = capabilities({ renameColumn: true, createIndex: true, dropIndex: true, + rebuildIndex: true, indexFilter: true, }); @@ -117,6 +123,7 @@ const duckdbCapabilities = capabilities({ renameColumn: true, createIndex: true, dropIndex: true, + rebuildIndex: true, }); const sqlserverCapabilities = capabilities({ @@ -126,6 +133,7 @@ const sqlserverCapabilities = capabilities({ dropColumn: true, createIndex: true, dropIndex: true, + rebuildIndex: true, indexType: true, indexInclude: true, indexFilter: true, @@ -144,6 +152,7 @@ const oracleCapabilities = capabilities({ comment: true, createIndex: true, dropIndex: true, + rebuildIndex: true, indexType: true, }); @@ -160,6 +169,7 @@ const h2Capabilities = capabilities({ comment: true, createIndex: true, dropIndex: true, + rebuildIndex: true, }); const clickhouseCapabilities = capabilities({ diff --git a/apps/desktop/src/lib/tableStructureEditorSql.ts b/apps/desktop/src/lib/tableStructureEditorSql.ts index 2b03da8763..1ca0be0133 100644 --- a/apps/desktop/src/lib/tableStructureEditorSql.ts +++ b/apps/desktop/src/lib/tableStructureEditorSql.ts @@ -408,6 +408,99 @@ function buildDropIndexSql( return `DROP INDEX ${quoteIdent(databaseType, indexName)};`; } +function indexListChanged(next: string[], previous: string[] | null | undefined): boolean { + const nextClean = next.map(clean).filter(Boolean); + const previousClean = (previous ?? []).map(clean).filter(Boolean); + return nextClean.length !== previousClean.length || nextClean.some((value, i) => value !== previousClean[i]); +} + +function originalIndexType(index: EditableStructureIndex): string { + return clean(index.original?.index_type).toUpperCase(); +} + +function normalizedIndexType(index: EditableStructureIndex): string { + return clean(index.indexType).toUpperCase(); +} + +function hasExistingIndexChange(index: EditableStructureIndex): boolean { + const original = index.original; + if (!original) return false; + return ( + clean(index.name) !== clean(original.name) || + indexListChanged(index.columns, original.columns) || + index.isUnique !== original.is_unique || + normalizedIndexType(index) !== originalIndexType(index) || + indexListChanged(index.includedColumns, original.included_columns) || + clean(index.filter) !== clean(original.filter) || + clean(index.comment) !== clean(original.comment) + ); +} + +function mysqlIndexParts(indexType: string): { prefix: string; usingClause: string } { + const upper = indexType.toUpperCase(); + if (upper === "FULLTEXT" || upper === "SPATIAL") return { prefix: `${upper} `, usingClause: "" }; + if (upper === "RTREE") return { prefix: "SPATIAL ", usingClause: "" }; + if (upper === "BTREE" || upper === "HASH") return { prefix: "", usingClause: ` USING ${upper}` }; + return { prefix: "", usingClause: "" }; +} + +function buildCreateIndexStatements( + databaseType: StructureSqlFlavor, + table: string, + index: EditableStructureIndex, + warnings: string[], +): string[] { + const capabilities = getTableStructureCapabilities(databaseType as DatabaseType | undefined); + const name = clean(index.name); + const columns = index.columns.map(clean).filter(Boolean); + if (!name || columns.length === 0) return []; + + const unique = index.isUnique ? "UNIQUE " : ""; + const cols = columns.map((column) => quoteIdent(databaseType, column)).join(", "); + const idxType = normalizedIndexType(index); + let typePrefix = ""; + let usingClause = ""; + + if (idxType && capabilities.indexType) { + if (databaseType === "postgres") { + usingClause = ` USING ${idxType}`; + } else if (databaseType === "sqlserver") { + typePrefix = `${idxType} `; + } else if (databaseType === "mysql") { + const mysqlParts = mysqlIndexParts(idxType); + typePrefix = mysqlParts.prefix; + usingClause = mysqlParts.usingClause; + } else if (isOracleLike(databaseType) && idxType === "BITMAP") { + typePrefix = "BITMAP "; + } + } + + const incCols = index.includedColumns.map(clean).filter(Boolean); + const includeClause = + incCols.length > 0 && capabilities.indexInclude && (databaseType === "postgres" || databaseType === "sqlserver") + ? ` INCLUDE (${incCols.map((c) => quoteIdent(databaseType, c)).join(", ")})` + : ""; + const filter = clean(index.filter); + const supportsWhere = + capabilities.indexFilter && + (databaseType === "postgres" || databaseType === "sqlserver" || databaseType === "sqlite"); + const whereClause = filter && supportsWhere ? ` WHERE ${filter}` : ""; + const createSql = + databaseType === "postgres" + ? `CREATE ${unique}${typePrefix}INDEX ${quoteIdent(databaseType, name)} ON ${table}${usingClause} (${cols})${includeClause}${whereClause};` + : `CREATE ${unique}${typePrefix}INDEX ${quoteIdent(databaseType, name)}${usingClause} ON ${table} (${cols})${includeClause}${whereClause};`; + const statements = [createSql]; + + const comment = clean(index.comment); + if (comment && capabilities.indexComment && databaseType === "postgres") { + statements.push(`COMMENT ON INDEX ${quoteIdent(databaseType, name)} IS ${quoteString(comment)};`); + } else if (comment && capabilities.indexComment && databaseType !== "postgres") { + warnings.push(`Index comments are not supported for ${databaseType ?? "this database"} from this editor.`); + } + + return statements; +} + function buildIndexSql(options: BuildTableStructureChangeSqlOptions, warnings: string[]): string[] { const databaseType = options.databaseType; const capabilities = getTableStructureCapabilities(databaseType); @@ -431,35 +524,26 @@ function buildIndexSql(options: BuildTableStructureChangeSqlOptions, warnings: s continue; } - if (index.original) continue; - const name = clean(index.name); - const columns = index.columns.map(clean).filter(Boolean); - if (!name || columns.length === 0) continue; + if (index.original) { + if (!hasExistingIndexChange(index)) continue; + if (!capabilities.rebuildIndex || !capabilities.dropIndex || !capabilities.createIndex) { + warnings.push(`Editing existing indexes is not supported for ${databaseLabel} from this editor.`); + continue; + } + if (index.original.is_primary) { + warnings.push(`Primary index "${index.original.name}" cannot be edited from this editor.`); + continue; + } + statements.push(buildDropIndexSql(dialect, table, options.schema, index.original.name)); + statements.push(...buildCreateIndexStatements(dialect, table, index, warnings)); + continue; + } + if (!capabilities.createIndex) { warnings.push(`Creating indexes is not supported for ${databaseLabel} from this editor.`); continue; } - const unique = index.isUnique ? "UNIQUE " : ""; - const cols = columns.map((column) => quoteIdent(dialect, column)).join(", "); - const idxType = clean(index.indexType); - const usingClause = idxType && capabilities.indexType && dialect === "postgres" ? ` USING ${idxType}` : ""; - const typePrefix = idxType && capabilities.indexType && dialect === "sqlserver" ? `${idxType} ` : ""; - const incCols = index.includedColumns.map(clean).filter(Boolean); - const includeClause = - incCols.length > 0 && capabilities.indexInclude && (dialect === "postgres" || dialect === "sqlserver") - ? ` INCLUDE (${incCols.map((c) => quoteIdent(dialect, c)).join(", ")})` - : ""; - const filter = clean(index.filter); - const supportsWhere = - capabilities.indexFilter && (dialect === "postgres" || dialect === "sqlserver" || dialect === "sqlite"); - const whereClause = filter && supportsWhere ? ` WHERE ${filter}` : ""; - statements.push( - `CREATE ${unique}${typePrefix}INDEX ${quoteIdent(dialect, name)} ON ${table}${usingClause} (${cols})${includeClause}${whereClause};`, - ); - const comment = clean(index.comment); - if (comment && capabilities.indexComment && dialect === "postgres") { - statements.push(`COMMENT ON INDEX ${quoteIdent(dialect, name)} IS ${quoteString(comment)};`); - } + statements.push(...buildCreateIndexStatements(dialect, table, index, warnings)); } return statements; @@ -478,7 +562,9 @@ function validateDraft(options: BuildTableStructureChangeSqlOptions): string[] { if (key) names.add(key); } - for (const index of options.indexes.filter((idx) => !idx.markedForDrop && !idx.original)) { + for (const index of options.indexes.filter( + (idx) => !idx.markedForDrop && (!idx.original || hasExistingIndexChange(idx)), + )) { if (!clean(index.name)) warnings.push("Index name cannot be empty."); if (index.columns.map(clean).filter(Boolean).length === 0) { warnings.push(`Index "${index.name || "(new)"}" needs at least one column.`); @@ -564,21 +650,11 @@ export function buildCreateTableSql(options: BuildTableStructureChangeSqlOptions } for (const index of options.indexes.filter((idx) => !idx.markedForDrop && !idx.isPrimary)) { - const name = clean(index.name); - const columns = index.columns.map(clean).filter(Boolean); - if (!name || columns.length === 0) continue; if (!capabilities.createIndex) { warnings.push(`Creating indexes is not supported for ${databaseType ?? "this database"} from this editor.`); continue; } - const unique = index.isUnique ? "UNIQUE " : ""; - const cols = columns.map((c) => quoteIdent(dialect, c)).join(", "); - const idxType = clean(index.indexType); - const usingClause = idxType && capabilities.indexType && dialect === "postgres" ? ` USING ${idxType}` : ""; - const typePrefix = idxType && capabilities.indexType && dialect === "sqlserver" ? `${idxType} ` : ""; - statements.push( - `CREATE ${unique}${typePrefix}INDEX ${quoteIdent(dialect, name)} ON ${table}${usingClause} (${cols});`, - ); + statements.push(...buildCreateIndexStatements(dialect, table, index, warnings)); } return { statements, warnings }; diff --git a/packages/app-tests/tableStructureCapabilities.test.ts b/packages/app-tests/tableStructureCapabilities.test.ts index 906543fe0c..93c20797dd 100644 --- a/packages/app-tests/tableStructureCapabilities.test.ts +++ b/packages/app-tests/tableStructureCapabilities.test.ts @@ -17,11 +17,24 @@ test("postgres-like databases expose safe structure editing capabilities", () => assert.equal(caps.comment, true, `${dbType} should support comments`); assert.equal(caps.createIndex, true, `${dbType} should create indexes`); assert.equal(caps.dropIndex, true, `${dbType} should drop indexes`); + assert.equal(caps.rebuildIndex, true, `${dbType} should rebuild indexes`); assert.equal(caps.indexFilter, true, `${dbType} should support filtered indexes`); assert.equal(canEditTableStructure(dbType), true); } }); +test("mysql-like databases expose index rebuild and type capabilities", () => { + for (const dbType of ["mysql", "doris", "starrocks", "goldendb", "sundb"] as const) { + const caps = getTableStructureCapabilities(dbType); + assert.equal(caps.dialect, "mysql", `${dbType} should reuse mysql DDL`); + assert.equal(caps.createIndex, true); + assert.equal(caps.dropIndex, true); + assert.equal(caps.rebuildIndex, true); + assert.equal(caps.indexType, true); + assert.equal(canEditTableStructure(dbType), true); + } +}); + test("redshift reuses postgres column DDL but keeps indexes disabled", () => { const caps = getTableStructureCapabilities("redshift"); assert.equal(caps.dialect, "postgres"); @@ -33,6 +46,7 @@ test("redshift reuses postgres column DDL but keeps indexes disabled", () => { assert.equal(caps.comment, true); assert.equal(caps.createIndex, false); assert.equal(caps.dropIndex, false); + assert.equal(caps.rebuildIndex, false); assert.equal(caps.indexFilter, false); assert.equal(canEditTableStructure("redshift"), true); }); @@ -49,6 +63,7 @@ test("oracle-like databases expose oracle-compatible structure editing capabilit assert.equal(caps.comment, true); assert.equal(caps.createIndex, true); assert.equal(caps.dropIndex, true); + assert.equal(caps.rebuildIndex, true); assert.equal(canEditTableStructure(dbType), true); } }); @@ -66,6 +81,7 @@ test("limited analytic engines can open the editor for supported operations only assert.equal(clickhouse.alterDefault, true); assert.equal(clickhouse.comment, true); assert.equal(clickhouse.createIndex, false); + assert.equal(clickhouse.rebuildIndex, false); assert.equal(canEditTableStructure("clickhouse"), true); }); diff --git a/packages/app-tests/tableStructureEditorSql.test.ts b/packages/app-tests/tableStructureEditorSql.test.ts index 65573cea2a..628b17655c 100644 --- a/packages/app-tests/tableStructureEditorSql.test.ts +++ b/packages/app-tests/tableStructureEditorSql.test.ts @@ -119,19 +119,17 @@ test("builds PostgreSQL rename, type, default, comment, and index statements", ( }, }), ], - indexes: [ - index({ id: "new", name: "idx_accounts_status", columns: ["account_status"] }), - ], + indexes: [index({ id: "new", name: "idx_accounts_status", columns: ["account_status"] })], }); assert.deepEqual(result.warnings, []); assert.deepEqual(result.statements, [ - "ALTER TABLE \"public\".\"accounts\" RENAME COLUMN \"status\" TO \"account_status\";", - "ALTER TABLE \"public\".\"accounts\" ALTER COLUMN \"account_status\" TYPE text;", - "ALTER TABLE \"public\".\"accounts\" ALTER COLUMN \"account_status\" SET NOT NULL;", - "ALTER TABLE \"public\".\"accounts\" ALTER COLUMN \"account_status\" SET DEFAULT 'active';", - "COMMENT ON COLUMN \"public\".\"accounts\".\"account_status\" IS 'Current status';", - "CREATE INDEX \"idx_accounts_status\" ON \"public\".\"accounts\" (\"account_status\");", + 'ALTER TABLE "public"."accounts" RENAME COLUMN "status" TO "account_status";', + 'ALTER TABLE "public"."accounts" ALTER COLUMN "account_status" TYPE text;', + 'ALTER TABLE "public"."accounts" ALTER COLUMN "account_status" SET NOT NULL;', + 'ALTER TABLE "public"."accounts" ALTER COLUMN "account_status" SET DEFAULT \'active\';', + 'COMMENT ON COLUMN "public"."accounts"."account_status" IS \'Current status\';', + 'CREATE INDEX "idx_accounts_status" ON "public"."accounts" ("account_status");', ]); }); @@ -159,11 +157,9 @@ test("warns when SQLite cannot safely alter existing column attributes", () => { indexes: [], }); - assert.deepEqual(result.statements, [ - "ALTER TABLE \"notes\" ADD COLUMN \"body\" text;", - ]); + assert.deepEqual(result.statements, ['ALTER TABLE "notes" ADD COLUMN "body" text;']); assert.deepEqual(result.warnings, [ - "SQLite cannot safely alter existing column \"title\" without rebuilding the table.", + 'SQLite cannot safely alter existing column "title" without rebuilding the table.', ]); }); @@ -172,12 +168,8 @@ test("quotes SQL Server table, column, and index names with brackets", () => { databaseType: "sqlserver", schema: "dbo", tableName: "users", - columns: [ - column({ id: "email", name: "email", dataType: "nvarchar(255)", isNullable: false }), - ], - indexes: [ - index({ id: "idx", name: "idx_users_email", columns: ["email"] }), - ], + columns: [column({ id: "email", name: "email", dataType: "nvarchar(255)", isNullable: false })], + indexes: [index({ id: "idx", name: "idx_users_email", columns: ["email"] })], }); assert.deepEqual(result.warnings, []); @@ -244,9 +236,7 @@ test("PostgreSQL index with USING clause (index type)", () => { }); assert.deepEqual(result.warnings, []); - assert.deepEqual(result.statements, [ - 'CREATE INDEX "idx_docs_body" ON "public"."docs" USING GIN ("body");', - ]); + assert.deepEqual(result.statements, ['CREATE INDEX "idx_docs_body" ON "public"."docs" USING GIN ("body");']); }); test("PostgreSQL index with WHERE filter", () => { @@ -361,9 +351,7 @@ test("SQL Server index with type prefix", () => { }); assert.deepEqual(result.warnings, []); - assert.deepEqual(result.statements, [ - "CREATE CLUSTERED INDEX [idx_logs_message] ON [dbo].[logs] ([message]);", - ]); + assert.deepEqual(result.statements, ["CREATE CLUSTERED INDEX [idx_logs_message] ON [dbo].[logs] ([message]);"]); }); test("SQL Server index with INCLUDE clause", () => { @@ -411,7 +399,7 @@ test("SQL Server index with type + include combined", () => { ]); }); -test("MySQL index omits USING, type prefix, INCLUDE, and WHERE (unsupported)", () => { +test("MySQL index uses BTREE/HASH with USING and still omits unsupported INCLUDE and WHERE", () => { const result = buildTableStructureChangeSql({ databaseType: "mysql", tableName: "orders", @@ -429,9 +417,102 @@ test("MySQL index omits USING, type prefix, INCLUDE, and WHERE (unsupported)", ( ], }); + assert.deepEqual(result.warnings, []); + assert.deepEqual(result.statements, ["CREATE INDEX `idx_orders_status` USING BTREE ON `orders` (`status`);"]); +}); + +test("MySQL fulltext and spatial index types are emitted as index prefixes", () => { + const result = buildTableStructureChangeSql({ + databaseType: "mysql", + tableName: "docs", + columns: [], + indexes: [ + index({ + id: "fulltext", + name: "idx_docs_body", + columns: ["body"], + indexType: "FULLTEXT", + }), + index({ + id: "spatial", + name: "idx_docs_location", + columns: ["location"], + indexType: "SPATIAL", + }), + index({ + id: "rtree", + name: "idx_docs_shape", + columns: ["shape"], + indexType: "RTREE", + }), + ], + }); + assert.deepEqual(result.warnings, []); assert.deepEqual(result.statements, [ - "CREATE INDEX `idx_orders_status` ON `orders` (`status`);", + "CREATE FULLTEXT INDEX `idx_docs_body` ON `docs` (`body`);", + "CREATE SPATIAL INDEX `idx_docs_location` ON `docs` (`location`);", + "CREATE SPATIAL INDEX `idx_docs_shape` ON `docs` (`shape`);", + ]); +}); + +test("changed existing indexes are rebuilt with drop and create when supported", () => { + const result = buildTableStructureChangeSql({ + databaseType: "postgres", + schema: "public", + tableName: "users", + columns: [], + indexes: [ + index({ + id: "existing", + name: "uniq_users_email", + columns: ["email"], + isUnique: true, + indexType: "BTREE", + original: { + name: "idx_users_name", + columns: ["name"], + is_unique: false, + is_primary: false, + index_type: "BTREE", + }, + }), + ], + }); + + assert.deepEqual(result.warnings, []); + assert.deepEqual(result.statements, [ + 'DROP INDEX "public"."idx_users_name";', + 'CREATE UNIQUE INDEX "uniq_users_email" ON "public"."users" USING BTREE ("email");', + ]); +}); + +test("changed existing MySQL indexes are rebuilt with MySQL index type syntax", () => { + const result = buildTableStructureChangeSql({ + databaseType: "mysql", + tableName: "docs", + columns: [], + indexes: [ + index({ + id: "existing", + name: "idx_docs_body", + columns: ["body"], + indexType: "FULLTEXT", + original: { + name: "idx_docs_title", + columns: ["title"], + is_unique: false, + is_primary: false, + index_type: "BTREE", + }, + }), + ], + }); + + assert.deepEqual(result.warnings, []); + assert.deepEqual(result.statements, [ + "DROP INDEX `idx_docs_title` ON `docs`;", + "CREATE FULLTEXT INDEX `idx_docs_body` ON `docs` (`body`);", ]); }); @@ -462,15 +543,10 @@ test("index with empty name and columns produces warnings and no statements", () schema: "public", tableName: "users", columns: [], - indexes: [ - index({ id: "empty", name: "", columns: [] }), - ], + indexes: [index({ id: "empty", name: "", columns: [] })], }); - assert.deepEqual(result.warnings, [ - 'Index name cannot be empty.', - 'Index "(new)" needs at least one column.', - ]); + assert.deepEqual(result.warnings, ["Index name cannot be empty.", 'Index "(new)" needs at least one column.']); assert.deepEqual(result.statements, []); }); @@ -654,7 +730,7 @@ test("Redshift skips unsupported index operations while keeping column DDL", () }); assert.deepEqual(result.statements, ['ALTER TABLE "public"."events" ADD COLUMN "email" varchar(255);']); - assert.deepEqual(result.warnings, ['Creating indexes is not supported for redshift from this editor.']); + assert.deepEqual(result.warnings, ["Creating indexes is not supported for redshift from this editor."]); }); test("builds ClickHouse column DDL and skips indexes", () => { @@ -701,7 +777,7 @@ test("builds ClickHouse column DDL and skips indexes", () => { 'ALTER TABLE "events" RENAME COLUMN "kind" TO "event_kind";', 'ALTER TABLE "events" MODIFY COLUMN "event_kind" LowCardinality(String) DEFAULT \'view\';', ]); - assert.deepEqual(result.warnings, ['Creating indexes is not supported for clickhouse from this editor.']); + assert.deepEqual(result.warnings, ["Creating indexes is not supported for clickhouse from this editor."]); }); test("builds ClickHouse nullable and comment column changes", () => {