From 155eac66561a5038fd86b5dfde065649a04bdcf2 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Wed, 22 Jul 2026 12:54:32 +0800 Subject: [PATCH] fix(iris): prevent unsupported comment statements --- .../table/tableStructureCapabilities.spec.ts | 15 +++ .../lib/table/tableStructureCapabilities.ts | 8 +- .../src/table_structure_sql/column_alter.rs | 51 ++++++- .../src/table_structure_sql/columns.rs | 35 ++++- .../src/table_structure_sql/comments.rs | 12 +- .../src/table_structure_sql/dialect.rs | 26 ++-- .../dbx-core/src/table_structure_sql/tests.rs | 124 ++++++++++++++++++ 7 files changed, 249 insertions(+), 22 deletions(-) diff --git a/apps/desktop/src/lib/__tests__/table/tableStructureCapabilities.spec.ts b/apps/desktop/src/lib/__tests__/table/tableStructureCapabilities.spec.ts index a0867c949e..68e96a49f9 100644 --- a/apps/desktop/src/lib/__tests__/table/tableStructureCapabilities.spec.ts +++ b/apps/desktop/src/lib/__tests__/table/tableStructureCapabilities.spec.ts @@ -28,6 +28,21 @@ describe("tableStructureCapabilities", () => { expect(getTableStructureCapabilities("postgres", "postgres").alterStrategy).toBe("direct"); }); + it("disables persisted comment editing for IRIS without disabling other structure changes", () => { + expect(getTableStructureCapabilities("iris", "iris")).toMatchObject({ + comment: false, + addColumn: true, + dropColumn: true, + renameColumn: true, + alterType: true, + alterNullability: true, + alterDefault: true, + }); + expect(getTableStructureCapabilities("oracle", "oracle").comment).toBe(true); + expect(getTableStructureCapabilities("oceanbase-oracle", "oceanbase-oracle").comment).toBe(true); + expect(getTableStructureCapabilities("dameng", "dameng").comment).toBe(true); + }); + it("uses local-only column reordering for editable databases without physical reorder support", () => { for (const databaseType of ["sqlserver", "postgres", "sqlite", "oracle", "dameng", "duckdb", "informix"] as const) { expect(supportsLocalTableColumnReorder(databaseType, databaseType)).toBe(true); diff --git a/apps/desktop/src/lib/table/tableStructureCapabilities.ts b/apps/desktop/src/lib/table/tableStructureCapabilities.ts index 656142e442..2abe17f19f 100644 --- a/apps/desktop/src/lib/table/tableStructureCapabilities.ts +++ b/apps/desktop/src/lib/table/tableStructureCapabilities.ts @@ -190,6 +190,12 @@ const oracleCapabilities = capabilities({ indexType: true, }); +const irisCapabilities = capabilities({ + ...oracleCapabilities, + // IRIS exposes %DESCRIPTION at definition time but cannot alter persisted descriptions. + comment: false, +}); + const h2Capabilities = capabilities({ dialect: "h2", createTable: true, @@ -318,7 +324,7 @@ const capabilityByType: Partial oracle: oracleCapabilities, dameng: oracleCapabilities, "oceanbase-oracle": oracleCapabilities, - iris: oracleCapabilities, + iris: irisCapabilities, yashandb: oracleCapabilities, xugu: oracleCapabilities, h2: h2Capabilities, diff --git a/crates/dbx-core/src/table_structure_sql/column_alter.rs b/crates/dbx-core/src/table_structure_sql/column_alter.rs index 1d8cc301d0..ad787bc8d3 100644 --- a/crates/dbx-core/src/table_structure_sql/column_alter.rs +++ b/crates/dbx-core/src/table_structure_sql/column_alter.rs @@ -51,14 +51,22 @@ pub fn build_single_column_alter_sql(options: SingleColumnAlterSqlOptions) -> Ta } let has_rename = options.column.name != original.name; + let has_comment_change = clean(&options.column.comment) != original_comment(&options.column); let has_attribute_change = options.column.data_type.trim() != original.data_type.trim() || options.column.is_nullable != original.is_nullable || normalize_default(Some(&options.column.default_value)) != original_default(&options.column) - || clean(&options.column.comment) != original_comment(&options.column) + || (has_comment_change && capabilities.comment) || (is_mysql_character_data_type(&options.column.data_type) && (options.column.character_set.trim() != original.character_set.as_deref().unwrap_or("") || options.column.collation.trim() != original.collation.as_deref().unwrap_or(""))); + if has_comment_change && !capabilities.comment { + warnings.push(format!( + "Column comments are not supported for {database_label} from this editor; the comment change for \"{}\" was ignored.", + original.name + )); + } + if has_rename && !capabilities.rename_column { warnings.push(format!("Renaming columns is not supported for {database_label} from this editor.")); } @@ -71,13 +79,18 @@ pub fn build_single_column_alter_sql(options: SingleColumnAlterSqlOptions) -> Ta { return TableStructureSqlResult { statements, warnings }; } + if !has_rename && !has_attribute_change && !has_column_extra_change(&options.column) { + return TableStructureSqlResult { statements, warnings }; + } match dialect { StructureDialect::Mysql => statements.extend(build_mysql_existing_column_sql(&table, &options.column, "")), StructureDialect::Doris => statements.extend(build_doris_existing_column_sql(&table, &options.column, "")), StructureDialect::Postgres => statements.extend(build_postgres_existing_column_sql(&table, &options.column)), StructureDialect::Oracle | StructureDialect::Dameng => { - if options.database_type == Some(crate::models::connection::DatabaseType::Xugu) { + if options.database_type == Some(crate::models::connection::DatabaseType::Iris) { + statements.extend(build_iris_existing_column_sql(&table, &options.column)); + } else if options.database_type == Some(crate::models::connection::DatabaseType::Xugu) { statements.extend(build_xugu_existing_column_sql(&table, &options.column)); } else { statements.extend(build_oracle_like_existing_column_sql(dialect, &table, &options.column)) @@ -462,6 +475,40 @@ pub(super) fn build_oracle_like_existing_column_sql( statements } +pub(super) fn build_iris_existing_column_sql(table: &str, column: &EditableStructureColumn) -> Vec { + let Some(original) = &column.original else { + return Vec::new(); + }; + let dialect = StructureDialect::Oracle; + let mut statements = Vec::new(); + let mut current_name = original.name.clone(); + if column.name != original.name { + statements.push(format!( + "ALTER TABLE {table} ALTER COLUMN {} RENAME {};", + quote_ident(dialect, &original.name), + quote_ident(dialect, &column.name) + )); + current_name = column.name.clone(); + } + let type_changed = column.data_type.trim() != original.data_type.trim(); + let nullable_changed = column.is_nullable != original.is_nullable; + let default_changed = normalize_default(Some(&column.default_value)) != original_default(column); + if type_changed || nullable_changed || default_changed { + let mut parts = vec![quote_ident(dialect, ¤t_name), column_data_type(dialect, column)]; + let default_value = normalize_default(Some(&column.default_value)); + if !default_value.is_empty() { + parts.push(format!("DEFAULT {}", format_default_for_sql(dialect, &column.data_type, &default_value))); + } else if default_changed { + parts.push("DEFAULT NULL".to_string()); + } + if nullable_changed { + parts.push(if column.is_nullable { "NULL".to_string() } else { "NOT NULL".to_string() }); + } + statements.push(format!("ALTER TABLE {table} MODIFY ({});", parts.join(" "))); + } + statements +} + pub(super) fn build_sqlserver_existing_column_sql( table: &str, column: &EditableStructureColumn, diff --git a/crates/dbx-core/src/table_structure_sql/columns.rs b/crates/dbx-core/src/table_structure_sql/columns.rs index b73943b0e1..620be7c6b8 100644 --- a/crates/dbx-core/src/table_structure_sql/columns.rs +++ b/crates/dbx-core/src/table_structure_sql/columns.rs @@ -1,9 +1,9 @@ use super::column_alter::{ build_clickhouse_existing_column_sql, build_doris_existing_column_sql, build_h2_existing_column_sql, - build_informix_existing_column_sql, build_mysql_existing_column_sql, build_oracle_like_existing_column_sql, - build_postgres_existing_column_sql, build_questdb_existing_column_sql, build_sqlite_existing_column_sql, - build_sqlserver_existing_column_sql, build_xugu_existing_column_sql, has_column_extra_change, - has_existing_column_attribute_change, + build_informix_existing_column_sql, build_iris_existing_column_sql, build_mysql_existing_column_sql, + build_oracle_like_existing_column_sql, build_postgres_existing_column_sql, build_questdb_existing_column_sql, + build_sqlite_existing_column_sql, build_sqlserver_existing_column_sql, build_xugu_existing_column_sql, + has_column_extra_change, has_existing_column_attribute_change, }; use super::column_format::{ column_definition, has_dameng_identity, is_dameng_identity_compatible_type, is_mysql_character_data_type, @@ -106,9 +106,16 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu )); continue; } + if !capabilities.comment && !clean(&column.comment).is_empty() { + warnings.push(format!( + "Column comments are not supported for {database_label} from this editor; the comment for \"{}\" was ignored.", + column.name + )); + } statements.extend(build_add_column_sql( dialect, options.database_type, + capabilities.comment, &table, column, &position_clause, @@ -128,14 +135,21 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu } let original = column.original.as_ref().unwrap(); let has_rename = column.name != original.name; + let has_comment_change = clean(&column.comment) != original_comment(column); let has_attribute_change = column.data_type.trim() != original.data_type.trim() || column.is_nullable != original.is_nullable || normalize_default(Some(&column.default_value)) != original_default(column) - || clean(&column.comment) != original_comment(column) + || (has_comment_change && capabilities.comment) || (is_mysql_character_data_type(&column.data_type) && (column.character_set.trim() != original.character_set.as_deref().unwrap_or("") || column.collation.trim() != original.collation.as_deref().unwrap_or(""))) || has_column_extra_change(column); + if has_comment_change && !capabilities.comment { + warnings.push(format!( + "Column comments are not supported for {database_label} from this editor; the comment change for \"{}\" was ignored.", + original.name + )); + } if has_position_change && !capabilities.reorder_column { warnings.push(format!("Reordering columns is not supported for {database_label} from this editor.")); } @@ -151,6 +165,9 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu { continue; } + if !has_rename && !has_attribute_change && !has_position_change { + continue; + } match dialect { StructureDialect::Mysql => statements.extend(build_mysql_existing_column_sql( @@ -161,7 +178,9 @@ pub(super) fn build_column_sql(options: &TableStructureSqlOptions, warnings: &mu StructureDialect::Doris => statements.extend(build_doris_existing_column_sql(&table, column, "")), StructureDialect::Postgres => statements.extend(build_postgres_existing_column_sql(&table, column)), StructureDialect::Oracle | StructureDialect::Dameng => { - if options.database_type == Some(crate::models::connection::DatabaseType::Xugu) { + if options.database_type == Some(crate::models::connection::DatabaseType::Iris) { + statements.extend(build_iris_existing_column_sql(&table, column)); + } else if options.database_type == Some(crate::models::connection::DatabaseType::Xugu) { statements.extend(build_xugu_existing_column_sql(&table, column)); } else { statements.extend(build_oracle_like_existing_column_sql(dialect, &table, column)) @@ -285,6 +304,7 @@ fn is_sqlserver_identity_compatible_type(data_type: &str) -> bool { pub(super) fn build_add_column_sql( dialect: StructureDialect, database_type: Option, + supports_comments: bool, table: &str, column: &EditableStructureColumn, position_clause: &str, @@ -304,7 +324,8 @@ pub(super) fn build_add_column_sql( }; vec![format!("ALTER TABLE {table} {add_keyword} {definition}{position_clause};")] }; - if matches!(dialect, StructureDialect::Postgres | StructureDialect::Oracle | StructureDialect::Dameng) + if supports_comments + && matches!(dialect, StructureDialect::Postgres | StructureDialect::Oracle | StructureDialect::Dameng) && !clean(&column.comment).is_empty() { statements.push(format!( diff --git a/crates/dbx-core/src/table_structure_sql/comments.rs b/crates/dbx-core/src/table_structure_sql/comments.rs index dfd37271ef..ede1f23575 100644 --- a/crates/dbx-core/src/table_structure_sql/comments.rs +++ b/crates/dbx-core/src/table_structure_sql/comments.rs @@ -1,17 +1,21 @@ -use super::dialect::{capabilities_for, dialect_label, StructureDialect}; +use super::dialect::{capabilities_for, database_label, dialect_label, StructureDialect}; use super::types::TableStructureSqlOptions; use super::util::{clean, qualified_table, quote_string}; pub(super) fn build_table_comment_sql(options: &TableStructureSqlOptions, warnings: &mut Vec) -> Vec { let capabilities = capabilities_for(options.database_type); - if !capabilities.comment { - return Vec::new(); - } let new_comment = options.table_comment.as_deref().unwrap_or(""); let original_comment = options.original_table_comment.as_deref().unwrap_or(""); if clean(new_comment) == clean(original_comment) { return Vec::new(); } + if !capabilities.comment { + warnings.push(format!( + "Table comments are not supported for {} from this editor; the comment change was ignored.", + database_label(options.database_type) + )); + return Vec::new(); + } let dialect = capabilities.dialect; let table = qualified_table(dialect, options.schema.as_deref(), &options.table_name); let quoted = quote_string(&clean(new_comment)); diff --git a/crates/dbx-core/src/table_structure_sql/dialect.rs b/crates/dbx-core/src/table_structure_sql/dialect.rs index 0b46190f02..6086572b70 100644 --- a/crates/dbx-core/src/table_structure_sql/dialect.rs +++ b/crates/dbx-core/src/table_structure_sql/dialect.rs @@ -210,25 +210,35 @@ pub(super) fn capabilities_for(database_type: Option) -> TableStru index_type: true, ..base }, - Some( - DatabaseType::Oracle - | DatabaseType::OceanbaseOracle - | DatabaseType::Iris - | DatabaseType::Yashandb - | DatabaseType::Xugu, - ) => TableStructureCapabilities { + Some(DatabaseType::Iris) => TableStructureCapabilities { dialect: StructureDialect::Oracle, add_column: true, drop_column: true, rename_column: true, alter_existing_column: true, - comment: true, + // IRIS supports %DESCRIPTION while defining tables/columns, but cannot alter existing descriptions. + comment: false, create_index: true, drop_index: true, rebuild_index: true, index_type: true, ..base }, + Some(DatabaseType::Oracle | DatabaseType::OceanbaseOracle | DatabaseType::Yashandb | DatabaseType::Xugu) => { + TableStructureCapabilities { + dialect: StructureDialect::Oracle, + add_column: true, + drop_column: true, + rename_column: true, + alter_existing_column: true, + comment: true, + create_index: true, + drop_index: true, + rebuild_index: true, + index_type: true, + ..base + } + } Some(DatabaseType::H2) => TableStructureCapabilities { dialect: StructureDialect::H2, add_column: true, diff --git a/crates/dbx-core/src/table_structure_sql/tests.rs b/crates/dbx-core/src/table_structure_sql/tests.rs index c0a99300b7..feed9d7400 100644 --- a/crates/dbx-core/src/table_structure_sql/tests.rs +++ b/crates/dbx-core/src/table_structure_sql/tests.rs @@ -677,6 +677,130 @@ fn iris_drop_index_includes_table_name() { assert_eq!(result.statements, vec!["DROP INDEX \"index_id\" ON TABLE \"SQLUSER\".\"tb_a\";"]); } +#[test] +fn iris_ignores_comment_changes_but_keeps_supported_column_alters() { + let mut renamed = column("DISPLAY_NAME"); + renamed.data_type = "VARCHAR(40)".to_string(); + renamed.is_nullable = true; + renamed.default_value = "'after'".to_string(); + renamed.comment = "new description".to_string(); + renamed.original = Some(ColumnInfo { + name: "NAME".to_string(), + data_type: "VARCHAR(20)".to_string(), + is_nullable: false, + column_default: Some("before".to_string()), + is_primary_key: false, + extra: None, + comment: Some("old description".to_string()), + ..Default::default() + }); + let mut created_at = column("CREATED_AT"); + created_at.data_type = "TIMESTAMP".to_string(); + created_at.default_value = "CURRENT_TIMESTAMP".to_string(); + created_at.comment = "creation time".to_string(); + + let result = build_table_structure_change_sql(TableStructureSqlOptions { + database_type: Some(DatabaseType::Iris), + schema: Some("SQLUSER".to_string()), + table_name: "DBX_ISSUE_1678".to_string(), + columns: vec![renamed, created_at], + indexes: Vec::new(), + foreign_keys: Vec::new(), + triggers: Vec::new(), + table_comment: Some("new table description".to_string()), + original_table_comment: Some("old table description".to_string()), + }); + + assert_eq!( + result.statements, + vec![ + "ALTER TABLE \"SQLUSER\".\"DBX_ISSUE_1678\" ALTER COLUMN \"NAME\" RENAME \"DISPLAY_NAME\";", + "ALTER TABLE \"SQLUSER\".\"DBX_ISSUE_1678\" MODIFY (\"DISPLAY_NAME\" VARCHAR(40) DEFAULT 'after' NULL);", + "ALTER TABLE \"SQLUSER\".\"DBX_ISSUE_1678\" ADD (\"CREATED_AT\" TIMESTAMP DEFAULT CURRENT_TIMESTAMP);", + ] + ); + assert_eq!( + result.warnings, + vec![ + "Column comments are not supported for iris from this editor; the comment change for \"NAME\" was ignored.", + "Column comments are not supported for iris from this editor; the comment for \"CREATED_AT\" was ignored.", + "Table comments are not supported for iris from this editor; the comment change was ignored.", + ] + ); + assert!(result.statements.iter().all(|statement| !statement.contains("COMMENT ON"))); +} + +#[test] +fn iris_comment_only_change_returns_warning_without_sql() { + let mut name = column("NAME"); + name.comment = "new description".to_string(); + name.original = Some(ColumnInfo { + name: "NAME".to_string(), + data_type: "varchar(255)".to_string(), + is_nullable: true, + column_default: None, + is_primary_key: false, + extra: None, + comment: Some("old description".to_string()), + ..Default::default() + }); + + let result = build_single_column_alter_sql(SingleColumnAlterSqlOptions { + database_type: Some(DatabaseType::Iris), + schema: Some("SQLUSER".to_string()), + table_name: "DBX_ISSUE_1678".to_string(), + column: name, + }); + + assert!(result.statements.is_empty()); + assert_eq!( + result.warnings, + vec![ + "Column comments are not supported for iris from this editor; the comment change for \"NAME\" was ignored." + ] + ); +} + +#[test] +fn oracle_compatible_databases_keep_comment_on_sql() { + for database_type in [DatabaseType::Oracle, DatabaseType::OceanbaseOracle, DatabaseType::Dameng] { + let mut name = column("NAME"); + name.comment = "new description".to_string(); + name.original = Some(ColumnInfo { + name: "NAME".to_string(), + data_type: "varchar(255)".to_string(), + is_nullable: true, + column_default: None, + is_primary_key: false, + extra: None, + comment: Some("old description".to_string()), + ..Default::default() + }); + + let result = build_table_structure_change_sql(TableStructureSqlOptions { + database_type: Some(database_type), + schema: Some("APP".to_string()), + table_name: "USERS".to_string(), + columns: vec![name], + indexes: Vec::new(), + foreign_keys: Vec::new(), + triggers: Vec::new(), + table_comment: Some("new table description".to_string()), + original_table_comment: Some("old table description".to_string()), + }); + + assert_eq!(result.warnings, Vec::::new(), "{database_type:?}"); + assert_eq!( + result.statements, + vec![ + "COMMENT ON COLUMN \"APP\".\"USERS\".\"NAME\" IS 'new description';", + "COMMENT ON TABLE \"APP\".\"USERS\" IS 'new table description';", + ], + "{database_type:?}" + ); + } +} + #[test] fn mysql_create_index_with_comment() { let mut col = column("name");