From ecac4800c43c6090a7c50f95554dd44b6cff07f6 Mon Sep 17 00:00:00 2001 From: zipg Date: Fri, 31 Jul 2026 19:33:43 +0800 Subject: [PATCH] fix(postgres): preserve copied table comments safely --- .../src/components/objects/ObjectBrowser.vue | 15 ++- .../objects/ObjectBrowserClipboard.spec.ts | 6 + .../src/components/sidebar/ConnectionTree.vue | 1 + .../sidebar/SidebarTreeRuntimeHost.vue | 22 +++- .../__tests__/crossDatabaseTablePaste.spec.ts | 7 ++ .../sidebar/sidebarTreeDialogState.ts | 2 +- .../lib/__tests__/database/dbAdminSql.spec.ts | 13 +- apps/desktop/src/lib/database/dbAdminSql.ts | 5 + apps/desktop/src/stores/connectionStore.ts | 1 + crates/dbx-core/src/db_admin_sql.rs | 113 ++++++++++++++++-- 10 files changed, 166 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/components/objects/ObjectBrowser.vue b/apps/desktop/src/components/objects/ObjectBrowser.vue index 1ac98d39b5..30cc49e3db 100644 --- a/apps/desktop/src/components/objects/ObjectBrowser.vue +++ b/apps/desktop/src/components/objects/ObjectBrowser.vue @@ -71,6 +71,7 @@ import { buildEmptyTableSql, buildTruncateTableSql, collectDuplicateTableColumnComments, + duplicateTableStructureRequiresScript, supportsDropTableCascade, supportsTruncateTableCascade, type TableAdminSqlOptions, @@ -241,7 +242,7 @@ const batchEmptyPlan = ref[]>([]); // Paste table dialog state const showPasteDialog = ref(false); const pasteTableMode = ref("structure-and-data"); -const pasteTableEntries = ref<{ sourceName: string; targetName: string; schema?: string }[]>([]); +const pasteTableEntries = ref<{ sourceName: string; targetName: string; schema?: string; tableComment?: string | null }[]>([]); const pasteTableDataCopySupported = computed(() => supportsWholeRowTableDataCopy(effectiveDatabaseType.value)); const objectColumnWidths = ref>({ select: 34, @@ -1910,7 +1911,7 @@ function requestDuplicateStructure(row: ObjectBrowserRow) { showDuplicateDialog.value = true; } -async function buildDuplicateStructurePlan(sourceName: string, targetName: string, schema: string | undefined, sourceColumns?: ColumnInfo[]) { +async function buildDuplicateStructurePlan(sourceName: string, targetName: string, schema: string | undefined, tableComment?: string | null, sourceColumns?: ColumnInfo[]) { let columns = sourceColumns; if (effectiveDatabaseType.value === "dameng" && !columns) { try { @@ -1925,9 +1926,10 @@ async function buildDuplicateStructurePlan(sourceName: string, targetName: strin schema, sourceName, targetName, + tableComment, columnComments, }); - return { sql, sourceColumns: columns, executeAsScript: columnComments.length > 0 }; + return { sql, sourceColumns: columns, executeAsScript: duplicateTableStructureRequiresScript(sql) }; } function executeDuplicateStructurePlan(plan: { sql: string; executeAsScript: boolean }, schema: string | undefined) { @@ -1941,7 +1943,7 @@ async function confirmDuplicateStructure() { showDuplicateDialog.value = false; try { const schema = row.schema || selectedSchema.value; - const plan = await buildDuplicateStructurePlan(row.name, newName, schema); + const plan = await buildDuplicateStructurePlan(row.name, newName, schema, row.comment); const executed = await executeObjectBrowserSqlWithProductionGuard(plan.sql, () => executeDuplicateStructurePlan(plan, schema)); if (!executed) return; toast(t("contextMenu.duplicateStructureSuccess", { name: newName })); @@ -1962,6 +1964,7 @@ function copySelectedTablesToClipboard() { database: props.database, schema: normalizeObjectBrowserTableClipboardSchema(row.schema || selectedSchema.value), tableName: row.name, + tableComment: row.comment, })), }; toast(t("contextMenu.pasteTableClipboardUpdated"), 2000); @@ -2031,6 +2034,7 @@ function copySingleTableToClipboard(row: ObjectBrowserRow) { database: props.database, schema: normalizeObjectBrowserTableClipboardSchema(row.schema || selectedSchema.value), tableName: row.name, + tableComment: row.comment, }, ], }; @@ -2052,6 +2056,7 @@ function openPasteTableDialog() { sourceName: entry.tableName, targetName: `${entry.tableName}_copy`, schema: normalizeObjectBrowserTableClipboardSchema(entry.schema, entry.database, entry.connectionId), + tableComment: entry.tableComment, })); showPasteDialog.value = true; } @@ -2090,7 +2095,7 @@ async function confirmPasteTable() { try { let sourceColumns: ColumnInfo[] | undefined; if (mode === "structure-and-data" || mode === "structure-only") { - const plan = await buildDuplicateStructurePlan(entry.sourceName, targetName, schema, sourceColumns); + const plan = await buildDuplicateStructurePlan(entry.sourceName, targetName, schema, entry.tableComment, sourceColumns); sourceColumns = plan.sourceColumns; const executed = await executeObjectBrowserSqlWithProductionGuard(plan.sql, () => executeDuplicateStructurePlan(plan, schema)); if (!executed) { diff --git a/apps/desktop/src/components/objects/ObjectBrowserClipboard.spec.ts b/apps/desktop/src/components/objects/ObjectBrowserClipboard.spec.ts index 7a0d5dc941..328387f896 100644 --- a/apps/desktop/src/components/objects/ObjectBrowserClipboard.spec.ts +++ b/apps/desktop/src/components/objects/ObjectBrowserClipboard.spec.ts @@ -30,4 +30,10 @@ describe("ObjectBrowser table clipboard context menu", () => { expect(objectBrowserSource).toMatch(/if \(!executed\) \{[\s\S]*?pasteCancelled = true;[\s\S]*?break;/); expect(objectBrowserSource).toMatch(/if \(pasteCancelled\) \{[\s\S]*?if \(hasMutatedTable\)[\s\S]*?await reload\(\)[\s\S]*?refreshObjectListTreeNode[\s\S]*?pasteTableCancelledAfterPartial[\s\S]*?return;/); }); + + it("carries table comments through local copy and paste", () => { + expect(objectBrowserSource).toMatch(/tableName: row\.name,\s*tableComment: row\.comment/); + expect(objectBrowserSource).toMatch(/targetName: `\$\{entry\.tableName\}_copy`,[\s\S]*?tableComment: entry\.tableComment/); + expect(objectBrowserSource).toMatch(/buildDuplicateStructurePlan\(entry\.sourceName, targetName, schema, entry\.tableComment/); + }); }); diff --git a/apps/desktop/src/components/sidebar/ConnectionTree.vue b/apps/desktop/src/components/sidebar/ConnectionTree.vue index a220a8adeb..acd49f7cce 100644 --- a/apps/desktop/src/components/sidebar/ConnectionTree.vue +++ b/apps/desktop/src/components/sidebar/ConnectionTree.vue @@ -1597,6 +1597,7 @@ function copySelectedSidebarNames(): boolean { database: node.database!, schema: connectionObjectTreeNodeSchema(store.getConfig(node.connectionId!), node.database!, node.schema), tableName: node.label, + tableComment: node.comment, })), } : null; diff --git a/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue b/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue index e9118ba731..4e44d27d64 100644 --- a/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue +++ b/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue @@ -113,6 +113,7 @@ import { buildCopyTableDataSql, buildEmptyTableSql, buildTruncateTableSql, + duplicateTableStructureRequiresScript, supportsDropTableCascade, supportsTruncateTableCascade, supportsSchemaComment, @@ -921,6 +922,7 @@ function requestPasteTreeClipboard(): boolean { connectionId: entry.connectionId, database: entry.database, schema: normalizeTreeClipboardSchema(entry.connectionId, entry.database, entry.schema), + tableComment: entry.tableComment, })); showPasteDialog.value = true; return true; @@ -1421,6 +1423,7 @@ function updateTreeClipboardForNodes(nodes: TreeNode[]) { database: node.database, schema: normalizeTreeClipboardSchema(node.connectionId, node.database, node.schema), tableName: node.label, + tableComment: node.comment, })), }; } @@ -1923,7 +1926,7 @@ function openRenameObjectDialog() { showRenameObjectDialog.value = true; } -async function executeTreeNodeSqlWithProductionGuard(node: Pick, sql: string, options: { database?: string; schema?: string } = {}) { +async function executeTreeNodeSqlWithProductionGuard(node: Pick, sql: string, options: { database?: string; schema?: string; executeAsScript?: boolean } = {}) { if (!node.connectionId) return undefined; const database = options.database ?? node.database ?? ""; return executeWithProductionSqlGuard({ @@ -1931,7 +1934,7 @@ async function executeTreeNodeSqlWithProductionGuard(node: Pick api.executeQuery(node.connectionId!, database, sql, options.schema ?? node.schema), + execute: () => (options.executeAsScript ? api.executeScript(node.connectionId!, database, sql, options.schema ?? node.schema) : api.executeQuery(node.connectionId!, database, sql, options.schema ?? node.schema)), }); } @@ -2989,8 +2992,13 @@ async function confirmDuplicateStructure() { schema: node.schema, sourceName: node.label, targetName: newName, + tableComment: node.comment, + }); + await executeTreeNodeSqlWithProductionGuard(node, sql, { + database: node.database, + schema: node.schema, + executeAsScript: duplicateTableStructureRequiresScript(sql), }); - await executeTreeNodeSqlWithProductionGuard(node, sql, { database: node.database, schema: node.schema }); toast(t("contextMenu.duplicateStructureSuccess", { name: newName }), 3000); await refreshTableList(node); } catch (e: any) { @@ -3031,8 +3039,13 @@ async function confirmPasteTable() { schema: entry.schema, sourceName: entry.sourceName, targetName, + tableComment: entry.tableComment, + }); + const structureExecuted = await executeTreeNodeSqlWithProductionGuard(entry, structureSql, { + database: entry.database, + schema: entry.schema, + executeAsScript: duplicateTableStructureRequiresScript(structureSql), }); - const structureExecuted = await executeTreeNodeSqlWithProductionGuard(entry, structureSql, { database: entry.database, schema: entry.schema }); if (!structureExecuted) { pasteCancelled = true; break; @@ -3121,6 +3134,7 @@ function openPasteTableDialog() { connectionId: entry.connectionId, database: entry.database, schema: normalizeTreeClipboardSchema(entry.connectionId, entry.database, entry.schema), + tableComment: entry.tableComment, })); showPasteDialog.value = true; } diff --git a/apps/desktop/src/components/sidebar/__tests__/crossDatabaseTablePaste.spec.ts b/apps/desktop/src/components/sidebar/__tests__/crossDatabaseTablePaste.spec.ts index d6820d8b86..21970e77eb 100644 --- a/apps/desktop/src/components/sidebar/__tests__/crossDatabaseTablePaste.spec.ts +++ b/apps/desktop/src/components/sidebar/__tests__/crossDatabaseTablePaste.spec.ts @@ -17,4 +17,11 @@ describe("cross-database table paste", () => { expect(runtimeSource).toContain("if (canTransferTreeClipboardToCurrentNode()) return openTransferFromTreeClipboard();"); expect(runtimeSource).toContain("pasteTableMode.value = defaultPasteTableMode(currentDatabaseType());"); }); + + it("carries table comments through the local sidebar paste path", () => { + expect(runtimeSource).toMatch(/tableName: node\.label,\s*tableComment: node\.comment/); + expect(runtimeSource).toMatch(/targetName: `\$\{entry\.tableName\}_copy`,[\s\S]*?tableComment: entry\.tableComment/); + expect(runtimeSource).toMatch(/targetName,\s*tableComment: entry\.tableComment/); + expect(runtimeSource).toContain("executeAsScript: duplicateTableStructureRequiresScript(structureSql)"); + }); }); diff --git a/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts b/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts index 80c2d01cc8..540342f373 100644 --- a/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts +++ b/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts @@ -58,7 +58,7 @@ export const duplicateTableName = ref(""); export const duplicateStructureSource = ref(null); export const showPasteDialog = ref(false); export const pasteTableMode = ref("structure-and-data"); -export const pasteTableEntries = ref>([]); +export const pasteTableEntries = ref>([]); export const showCreateDatabaseDialog = ref(false); export const createDatabaseName = ref(""); export const createDatabaseCharset = ref("utf8mb4"); diff --git a/apps/desktop/src/lib/__tests__/database/dbAdminSql.spec.ts b/apps/desktop/src/lib/__tests__/database/dbAdminSql.spec.ts index b5e0b528cc..dd31779dd5 100644 --- a/apps/desktop/src/lib/__tests__/database/dbAdminSql.spec.ts +++ b/apps/desktop/src/lib/__tests__/database/dbAdminSql.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { collectDuplicateTableColumnComments } from "@/lib/database/dbAdminSql"; +import { collectDuplicateTableColumnComments, duplicateTableStructureRequiresScript } from "@/lib/database/dbAdminSql"; describe("collectDuplicateTableColumnComments", () => { it("preserves meaningful whitespace and excludes whitespace-only comments", () => { @@ -19,3 +19,14 @@ describe("collectDuplicateTableColumnComments", () => { ]); }); }); + +describe("duplicateTableStructureRequiresScript", () => { + it("detects generated table and column comment statements", () => { + expect(duplicateTableStructureRequiresScript('CREATE TABLE "copy" (LIKE "source" INCLUDING ALL);\nCOMMENT ON TABLE "copy" IS \'orders\';')).toBe(true); + expect(duplicateTableStructureRequiresScript('CREATE TABLE "copy" AS SELECT * FROM "source" WHERE 1=0;\nCOMMENT ON COLUMN "copy"."id" IS \'identifier\';')).toBe(true); + }); + + it("keeps single-statement structure copies on the query path", () => { + expect(duplicateTableStructureRequiresScript('CREATE TABLE "copy" (LIKE "source" INCLUDING ALL);')).toBe(false); + }); +}); diff --git a/apps/desktop/src/lib/database/dbAdminSql.ts b/apps/desktop/src/lib/database/dbAdminSql.ts index fabca86cbf..521707860f 100644 --- a/apps/desktop/src/lib/database/dbAdminSql.ts +++ b/apps/desktop/src/lib/database/dbAdminSql.ts @@ -55,6 +55,7 @@ export interface DuplicateTableStructureSqlOptions { schema?: string | null; sourceName: string; targetName: string; + tableComment?: string | null; columnComments?: Array<{ name: string; comment: string }>; } @@ -153,6 +154,10 @@ export function buildDuplicateTableStructureSql(options: DuplicateTableStructure return api.buildDuplicateTableStructureSql(options); } +export function duplicateTableStructureRequiresScript(sql: string): boolean { + return /;\s*\n\s*COMMENT ON (?:TABLE|COLUMN)\b/i.test(sql); +} + export function buildCopyTableDataSql(options: CopyTableDataSqlOptions): Promise { return api.buildCopyTableDataSql(options); } diff --git a/apps/desktop/src/stores/connectionStore.ts b/apps/desktop/src/stores/connectionStore.ts index 7797608549..5353bc3831 100644 --- a/apps/desktop/src/stores/connectionStore.ts +++ b/apps/desktop/src/stores/connectionStore.ts @@ -249,6 +249,7 @@ interface TreeClipboardTableEntry { database: string; schema?: string; tableName: string; + tableComment?: string | null; } interface TreeClipboardConnectionEntry { diff --git a/crates/dbx-core/src/db_admin_sql.rs b/crates/dbx-core/src/db_admin_sql.rs index 3c81dd677d..7d54c2c009 100644 --- a/crates/dbx-core/src/db_admin_sql.rs +++ b/crates/dbx-core/src/db_admin_sql.rs @@ -162,6 +162,8 @@ pub struct DuplicateTableStructureSqlOptions { pub schema: Option, pub source_name: String, pub target_name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub table_comment: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub column_comments: Vec, } @@ -609,20 +611,26 @@ pub fn build_duplicate_table_structure_sql(options: DuplicateTableStructureSqlOp format!("CREATE TABLE {target} AS SELECT * FROM {source} WHERE 0;") }; - if options.database_type != Some(DatabaseType::Dameng) { - return structure_sql; + let mut comment_sql = Vec::new(); + if let Some(database_type) = + options.database_type.filter(|database_type| supports_duplicate_table_comment(*database_type)) + { + if let Some(comment) = options.table_comment.as_deref().filter(|comment| !comment.trim().is_empty()) { + comment_sql.push(format!( + "COMMENT ON TABLE {target} IS {}", + quote_duplicate_table_comment(database_type, comment) + )); + } } - let comment_sql = options - .column_comments - .iter() - .filter_map(|column| { + if options.database_type == Some(DatabaseType::Dameng) { + comment_sql.extend(options.column_comments.iter().filter_map(|column| { if column.comment.trim().is_empty() { return None; } let column_name = quote_table_identifier(options.database_type, &column.name); Some(format!("COMMENT ON COLUMN {target}.{column_name} IS {}", quote_sql_string(&column.comment))) - }) - .collect::>(); + })); + } if comment_sql.is_empty() { return structure_sql; } @@ -768,6 +776,17 @@ fn is_postgres_like_structure_copy(database_type: DatabaseType) -> bool { ) } +fn supports_duplicate_table_comment(database_type: DatabaseType) -> bool { + matches!( + database_type, + DatabaseType::Postgres + | DatabaseType::Redshift + | DatabaseType::Gaussdb + | DatabaseType::Kwdb + | DatabaseType::OpenGauss + ) +} + fn uses_false_predicate_duplicate_structure(database_type: DatabaseType) -> bool { matches!(database_type, DatabaseType::Oracle | DatabaseType::Dameng | DatabaseType::Iris) } @@ -833,6 +852,36 @@ fn quote_sql_string(value: &str) -> String { format!("'{}'", value.replace('\'', "''")) } +fn quote_duplicate_table_comment(database_type: DatabaseType, value: &str) -> String { + if !value.contains('\\') && !value.chars().any(|character| character.is_ascii_control()) { + return quote_sql_string(value); + } + + let mut escaped = String::with_capacity(value.len()); + for character in value.chars() { + match character { + '\\' => escaped.push_str("\\\\"), + '\n' => escaped.push_str("\\n"), + '\r' => escaped.push_str("\\r"), + '\t' => escaped.push_str("\\t"), + '\x08' => escaped.push_str("\\b"), + '\x0c' => escaped.push_str("\\f"), + '\'' => escaped.push_str("\\'"), + character if character.is_ascii_control() => { + const HEX: &[u8; 16] = b"0123456789ABCDEF"; + let byte = character as u8; + escaped.push_str("\\x"); + escaped.push(HEX[(byte >> 4) as usize] as char); + escaped.push(HEX[(byte & 0x0F) as usize] as char); + } + character => escaped.push(character), + } + } + + let prefix = if database_type == DatabaseType::Redshift { "" } else { "E" }; + format!("{prefix}'{escaped}'") +} + fn comment_literal(value: Option<&str>) -> String { match value.map(str::trim).filter(|value| !value.is_empty()) { Some(value) => quote_sql_string(value), @@ -1481,6 +1530,7 @@ mod tests { schema: None, source_name: "users".to_string(), target_name: "users_copy".to_string(), + table_comment: None, column_comments: vec![], }), "CREATE TABLE `users_copy` LIKE `users`;" @@ -1491,16 +1541,29 @@ mod tests { schema: Some("public".to_string()), source_name: "users".to_string(), target_name: "users_copy".to_string(), + table_comment: None, column_comments: vec![], }), "CREATE TABLE \"public\".\"users_copy\" (LIKE \"public\".\"users\" INCLUDING ALL);" ); + assert_eq!( + build_duplicate_table_structure_sql(DuplicateTableStructureSqlOptions { + database_type: Some(DatabaseType::Postgres), + schema: Some("public".to_string()), + source_name: "customer_orders".to_string(), + target_name: "customer_orders_copy".to_string(), + table_comment: Some(" Customer's orders; archive ".to_string()), + column_comments: vec![], + }), + "CREATE TABLE \"public\".\"customer_orders_copy\" (LIKE \"public\".\"customer_orders\" INCLUDING ALL);\nCOMMENT ON TABLE \"public\".\"customer_orders_copy\" IS ' Customer''s orders; archive ';" + ); assert_eq!( build_duplicate_table_structure_sql(DuplicateTableStructureSqlOptions { database_type: Some(DatabaseType::Kwdb), schema: Some("public".to_string()), source_name: "users".to_string(), target_name: "users_copy".to_string(), + table_comment: None, column_comments: vec![], }), "CREATE TABLE \"public\".\"users_copy\" (LIKE \"public\".\"users\" INCLUDING ALL);" @@ -1511,6 +1574,7 @@ mod tests { schema: Some("dbo".to_string()), source_name: "users".to_string(), target_name: "users_copy".to_string(), + table_comment: None, column_comments: vec![], }), "SELECT TOP 0 * INTO [dbo].[users_copy] FROM [dbo].[users];" @@ -1521,6 +1585,7 @@ mod tests { schema: Some("HR".to_string()), source_name: "USERS".to_string(), target_name: "USERS_COPY".to_string(), + table_comment: None, column_comments: vec![], }), "CREATE TABLE \"HR\".\"USERS_COPY\" AS SELECT * FROM \"HR\".\"USERS\" WHERE 1=0" @@ -1530,6 +1595,7 @@ mod tests { schema: Some("APP".to_string()), source_name: "USERS".to_string(), target_name: "USERS_COPY".to_string(), + table_comment: None, column_comments: vec![ DuplicateTableColumnComment { name: "DISPLAY\"NAME".to_string(), @@ -1552,12 +1618,42 @@ mod tests { "COMMENT ON COLUMN \"APP\".\"USERS_COPY\".\"STATUS\" IS 'active '".to_string(), ] ); + for database_type in [ + DatabaseType::Postgres, + DatabaseType::Redshift, + DatabaseType::Gaussdb, + DatabaseType::Kwdb, + DatabaseType::OpenGauss, + ] { + let sql = build_duplicate_table_structure_sql(DuplicateTableStructureSqlOptions { + database_type: Some(database_type), + schema: Some("public".to_string()), + source_name: "source".to_string(), + target_name: "copy".to_string(), + table_comment: Some("owner\\'s; archive".to_string()), + column_comments: vec![], + }); + let expected_literal = if database_type == DatabaseType::Redshift { + "'owner\\\\\\'s; archive'" + } else { + "E'owner\\\\\\'s; archive'" + }; + assert!(sql.ends_with(&format!("COMMENT ON TABLE \"public\".\"copy\" IS {expected_literal};"))); + assert_eq!( + crate::sql::split_sql_statements_for_database(&sql, database_type), + vec![ + "CREATE TABLE \"public\".\"copy\" (LIKE \"public\".\"source\" INCLUDING ALL)".to_string(), + format!("COMMENT ON TABLE \"public\".\"copy\" IS {expected_literal}"), + ] + ); + } assert_eq!( build_duplicate_table_structure_sql(DuplicateTableStructureSqlOptions { database_type: Some(DatabaseType::Iris), schema: Some("SQLUSER".to_string()), source_name: "tb_a".to_string(), target_name: "tb_a_copy".to_string(), + table_comment: None, column_comments: vec![], }), "CREATE TABLE \"SQLUSER\".\"tb_a_copy\" AS SELECT * FROM \"SQLUSER\".\"tb_a\" WHERE 1=0" @@ -1568,6 +1664,7 @@ mod tests { schema: None, source_name: "users".to_string(), target_name: "users_copy".to_string(), + table_comment: Some("ignored by QuestDB".to_string()), column_comments: vec![], }), "CREATE TABLE `users_copy` (LIKE `users`);"