From 2225343e8aad100334c524f389a046473e871e4d Mon Sep 17 00:00:00 2001 From: Fody Jute Date: Sat, 4 Jul 2026 00:58:52 +0800 Subject: [PATCH] fix(dbx-core): preserve mysql cross-database targets --- crates/dbx-core/src/query_result_sql.rs | 2 +- crates/dbx-core/src/schema.rs | 81 ++++++++---- .../dbx-core/src/schema/providers/native.rs | 11 +- .../dbx-core/src/sql_dialect/identifiers.rs | 8 +- crates/dbx-core/src/sql_dialect/tests.rs | 7 +- .../tests/mysql_cross_database_editing.rs | 115 ++++++++++++++++++ 6 files changed, 197 insertions(+), 27 deletions(-) create mode 100644 crates/dbx-core/tests/mysql_cross_database_editing.rs diff --git a/crates/dbx-core/src/query_result_sql.rs b/crates/dbx-core/src/query_result_sql.rs index 9634082173..120f25046b 100644 --- a/crates/dbx-core/src/query_result_sql.rs +++ b/crates/dbx-core/src/query_result_sql.rs @@ -2198,7 +2198,7 @@ WHERE u.id = picked.id; // `add_outer_standard_limit` always includes OFFSET clause. assert_eq!( result.sql.unwrap(), - "SELECT * FROM (SELECT DISTINCT a, b, c FROM t LIMIT 500) \"dbx_page\" ORDER BY 1, 2, 3 LIMIT 100 OFFSET 0;" + "SELECT * FROM (SELECT DISTINCT a, b, c FROM t LIMIT 500) `dbx_page` ORDER BY 1, 2, 3 LIMIT 100 OFFSET 0;" ); } diff --git a/crates/dbx-core/src/schema.rs b/crates/dbx-core/src/schema.rs index 0c798485a8..ed7dc1051a 100644 --- a/crates/dbx-core/src/schema.rs +++ b/crates/dbx-core/src/schema.rs @@ -1902,8 +1902,12 @@ fn normalize_information_schema_table_type(table_type: &str) -> String { } } -fn mysql_table_metadata_catalog<'a>(database: &'a str, _schema: &str) -> &'a str { - database +fn mysql_table_metadata_catalog<'a>(database: &'a str, schema: &'a str) -> &'a str { + if schema.trim().is_empty() { + database + } else { + schema + } } fn quote_presto_like_identifier(identifier: &str) -> String { @@ -1924,13 +1928,14 @@ mod tests { use super::{ clickhouse_metadata_database, deduplicate_column_infos, filter_mysql_system_databases_for_config, filter_table_infos, filter_visible_schema_names, is_agent_postgres_metadata_fallback_config, - is_retryable_metadata_error, mysql_table_metadata_catalog, normalize_information_schema_table_type, - oracle_columns_from_query_result, oracle_columns_sql, oracle_object_statistics_dba_segments_sql, - oracle_object_statistics_from_query_result, oracle_object_statistics_rows_only_sql, - oracle_object_statistics_sql, oracle_object_statistics_user_segments_sql, - oracle_table_comment_from_query_result, oracle_table_comment_sql, oracle_table_comments_from_query_result, - oracle_table_comments_sql, presto_like_columns_from_query_result, presto_like_information_schema_columns_sql, - presto_like_information_schema_tables_sql, presto_like_tables_from_query_result, visible_schema_filter, + is_retryable_metadata_error, mysql_object_source_sql, mysql_table_metadata_catalog, + normalize_information_schema_table_type, oracle_columns_from_query_result, oracle_columns_sql, + oracle_object_statistics_dba_segments_sql, oracle_object_statistics_from_query_result, + oracle_object_statistics_rows_only_sql, oracle_object_statistics_sql, + oracle_object_statistics_user_segments_sql, oracle_table_comment_from_query_result, oracle_table_comment_sql, + oracle_table_comments_from_query_result, oracle_table_comments_sql, presto_like_columns_from_query_result, + presto_like_information_schema_columns_sql, presto_like_information_schema_tables_sql, + presto_like_tables_from_query_result, visible_schema_filter, }; #[cfg(feature = "duckdb-bundled")] use super::{ @@ -2005,9 +2010,29 @@ mod tests { } #[test] - fn mysql_table_child_metadata_uses_database_not_schema() { + fn mysql_table_child_metadata_prefers_schema_when_present() { assert_eq!(mysql_table_metadata_catalog("app_db", ""), "app_db"); - assert_eq!(mysql_table_metadata_catalog("app_db", "public"), "app_db"); + assert_eq!(mysql_table_metadata_catalog("app_db", "tenant_db"), "tenant_db"); + } + + #[test] + fn mysql_object_source_sql_qualifies_cross_database_objects() { + assert_eq!( + mysql_object_source_sql("tenant_db", "users_view", &db::ObjectSourceKind::View), + "SHOW CREATE VIEW `tenant_db`.`users_view`" + ); + assert_eq!( + mysql_object_source_sql("tenant_db", "sync_users", &db::ObjectSourceKind::Procedure), + "SHOW CREATE PROCEDURE `tenant_db`.`sync_users`" + ); + assert_eq!( + mysql_object_source_sql("tenant_db", "calc_score", &db::ObjectSourceKind::Function), + "SHOW CREATE FUNCTION `tenant_db`.`calc_score`" + ); + assert_eq!( + mysql_object_source_sql("", "users_view", &db::ObjectSourceKind::View), + "SHOW CREATE VIEW `users_view`" + ); } #[test] @@ -3562,7 +3587,8 @@ pub async fn get_columns_core( db::mysql::get_columns(p, metadata_database, table).await.map(deduplicate_column_infos) } PoolKind::Mysql(p, mode) => { - dispatch_mysql!(p, mode, db::mysql::get_columns, db::ob_oracle::get_columns, database, table) + let effective_db = mysql_table_metadata_catalog(database, schema); + dispatch_mysql!(p, mode, db::mysql::get_columns, db::ob_oracle::get_columns, effective_db, table) .map(deduplicate_column_infos) } PoolKind::Postgres(p) if db_config.as_ref().is_some_and(is_questdb_config) => { @@ -3968,7 +3994,7 @@ pub async fn get_table_ddl_core( let pool = connections.get(&pool_key).ok_or("Pool not found")?; match pool { - PoolKind::Mysql(p, _) => mysql_ddl(p, table).await, + PoolKind::Mysql(p, _) => mysql_ddl(p, mysql_table_metadata_catalog(database, schema), table).await, PoolKind::Postgres(p) if db_config.as_ref().is_some_and(is_opengauss_family_config) => { match opengauss_table_ddl(p, schema, table).await { Ok(ddl) => Ok(ddl), @@ -4058,6 +4084,14 @@ fn mysql_ident(value: &str) -> String { format!("`{}`", value.replace('`', "``")) } +fn mysql_qualified_name(database: &str, name: &str) -> String { + if database.trim().is_empty() { + mysql_ident(name) + } else { + format!("{}.{}", mysql_ident(database), mysql_ident(name)) + } +} + fn sqlite_object_type(kind: &db::ObjectSourceKind) -> &'static str { match kind { db::ObjectSourceKind::View | db::ObjectSourceKind::MaterializedView => "view", @@ -4217,11 +4251,12 @@ pub fn sqlite_object_source_sql(name: &str, kind: &db::ObjectSourceKind) -> Stri ) } -pub fn mysql_object_source_sql(name: &str, kind: &db::ObjectSourceKind) -> String { +pub fn mysql_object_source_sql(database: &str, name: &str, kind: &db::ObjectSourceKind) -> String { + let qualified_name = mysql_qualified_name(database, name); match kind { - db::ObjectSourceKind::View => format!("SHOW CREATE VIEW {}", mysql_ident(name)), - db::ObjectSourceKind::Procedure => format!("SHOW CREATE PROCEDURE {}", mysql_ident(name)), - db::ObjectSourceKind::Function => format!("SHOW CREATE FUNCTION {}", mysql_ident(name)), + db::ObjectSourceKind::View => format!("SHOW CREATE VIEW {qualified_name}"), + db::ObjectSourceKind::Procedure => format!("SHOW CREATE PROCEDURE {qualified_name}"), + db::ObjectSourceKind::Function => format!("SHOW CREATE FUNCTION {qualified_name}"), db::ObjectSourceKind::Sequence | db::ObjectSourceKind::Package | db::ObjectSourceKind::PackageBody @@ -4251,11 +4286,12 @@ fn first_string_cell(result: db::QueryResult) -> Result { async fn mysql_object_source( pool: &db::mysql::MySqlPool, + database: &str, name: &str, kind: &db::ObjectSourceKind, ) -> Result { use mysql_async::prelude::*; - let sql = mysql_object_source_sql(name, kind); + let sql = mysql_object_source_sql(database, name, kind); let mut conn = db::mysql::get_conn_with_timeout(pool, db::connection_timeout()).await?; let result = conn.query_iter(&sql).await.map_err(|e| e.to_string())?; let rows: Vec = result.collect_and_drop().await.map_err(|e| e.to_string())?; @@ -4351,7 +4387,10 @@ async fn get_object_source_once( } } else { match connections.get(&pool_key).ok_or("Pool not found")? { - PoolKind::Mysql(pool, _) => mysql_object_source(pool, name, &object_type).await?, + PoolKind::Mysql(pool, _) => { + mysql_object_source(pool, mysql_table_metadata_catalog(database, schema), name, &object_type) + .await? + } PoolKind::Postgres(pool) if db_config.as_ref().is_some_and(is_questdb_config) => { // only view db::questdb::questdb_object_source(pool, name).await? @@ -4910,9 +4949,9 @@ mod ddl_tests { } } -pub async fn mysql_ddl(pool: &db::mysql::MySqlPool, table: &str) -> Result { +pub async fn mysql_ddl(pool: &db::mysql::MySqlPool, database: &str, table: &str) -> Result { use mysql_async::prelude::*; - let sql = format!("SHOW CREATE TABLE `{}`", table.replace('`', "``")); + let sql = format!("SHOW CREATE TABLE {}", mysql_qualified_name(database, table)); // Use the health-checked getter so a stale pooled connection (server closed // it after an idle timeout, NAT/firewall dropped the TCP state, etc.) is // detected and replaced before issuing the query. Without this, the first diff --git a/crates/dbx-core/src/schema/providers/native.rs b/crates/dbx-core/src/schema/providers/native.rs index f70ff7a026..77b0b39af8 100644 --- a/crates/dbx-core/src/schema/providers/native.rs +++ b/crates/dbx-core/src/schema/providers/native.rs @@ -218,7 +218,7 @@ pub(in crate::schema) async fn table_ddl( table: &str, ) -> Result { match pool { - PoolKind::Mysql(p, _) => super::super::mysql_ddl(p, table).await, + PoolKind::Mysql(p, _) => super::super::mysql_ddl(p, schema, table).await, PoolKind::Postgres(p) if config.is_some_and(is_opengauss_family_config) => { match super::super::opengauss_table_ddl(p, schema, table).await { Ok(ddl) => Ok(ddl), @@ -247,7 +247,14 @@ pub(in crate::schema) async fn object_source( object_type: &db::ObjectSourceKind, ) -> Result, String> { match pool { - PoolKind::Mysql(pool, _) => super::super::mysql_object_source(pool, name, object_type).await.map(Some), + PoolKind::Mysql(pool, _) => super::super::mysql_object_source( + pool, + super::super::mysql_table_metadata_catalog(database, schema), + name, + object_type, + ) + .await + .map(Some), PoolKind::Postgres(pool) => { super::super::postgres_object_source(pool, schema, name, object_type).await.map(Some) } diff --git a/crates/dbx-core/src/sql_dialect/identifiers.rs b/crates/dbx-core/src/sql_dialect/identifiers.rs index 9eeeb95805..52fde76c9d 100644 --- a/crates/dbx-core/src/sql_dialect/identifiers.rs +++ b/crates/dbx-core/src/sql_dialect/identifiers.rs @@ -48,7 +48,12 @@ pub fn qualified_table_name(database_type: Option, schema: Option< } return table_name; } - if database_type.is_some_and(is_schema_aware) + let supports_qualifier = database_type.is_some_and(is_schema_aware) + || matches!( + database_type, + Some(DatabaseType::Mysql | DatabaseType::Goldendb | DatabaseType::StarRocks | DatabaseType::Doris) + ); + if supports_qualifier && database_type != Some(DatabaseType::Jdbc) && schema.is_some_and(|schema| !schema.trim().is_empty()) { @@ -76,6 +81,7 @@ pub fn quote_table_identifier(database_type: Option, name: &str) - Some( DatabaseType::Mysql | DatabaseType::ClickHouse + | DatabaseType::Doris | DatabaseType::Goldendb | DatabaseType::StarRocks | DatabaseType::ManticoreSearch diff --git a/crates/dbx-core/src/sql_dialect/tests.rs b/crates/dbx-core/src/sql_dialect/tests.rs index bd9eb99f1e..e7ca1aae67 100644 --- a/crates/dbx-core/src/sql_dialect/tests.rs +++ b/crates/dbx-core/src/sql_dialect/tests.rs @@ -16,6 +16,7 @@ fn transfer_identifier_policy_preserves_legacy_output() { fn quotes_identifiers_by_database_type() { assert_eq!(quote_table_identifier(Some(DatabaseType::Mysql), "user`name"), "`user``name`"); assert_eq!(quote_table_identifier(Some(DatabaseType::ClickHouse), "user`name"), "`user``name`"); + assert_eq!(quote_table_identifier(Some(DatabaseType::Doris), "user`name"), "`user``name`"); assert_eq!(quote_table_identifier(Some(DatabaseType::Goldendb), "user`name"), "`user``name`"); assert_eq!(quote_table_identifier(Some(DatabaseType::StarRocks), "user`name"), "`user``name`"); assert_eq!(quote_table_identifier(Some(DatabaseType::SqlServer), "user]name"), "[user]]name]"); @@ -30,8 +31,10 @@ fn quotes_identifiers_by_database_type() { fn qualifies_schema_only_for_schema_aware_databases() { assert_eq!(qualified_table_name(Some(DatabaseType::Postgres), Some("public"), "users"), "\"public\".\"users\""); assert_eq!(qualified_table_name(Some(DatabaseType::Kwdb), Some("public"), "users"), "\"public\".\"users\""); - assert_eq!(qualified_table_name(Some(DatabaseType::Mysql), Some("public"), "users"), "`users`"); - assert_eq!(qualified_table_name(Some(DatabaseType::Goldendb), Some("public"), "users"), "`users`"); + assert_eq!(qualified_table_name(Some(DatabaseType::Mysql), Some("public"), "users"), "`public`.`users`"); + assert_eq!(qualified_table_name(Some(DatabaseType::Goldendb), Some("public"), "users"), "`public`.`users`"); + assert_eq!(qualified_table_name(Some(DatabaseType::StarRocks), Some("warehouse"), "users"), "`warehouse`.`users`"); + assert_eq!(qualified_table_name(Some(DatabaseType::Doris), Some("warehouse"), "users"), "`warehouse`.`users`"); assert_eq!(qualified_table_name(Some(DatabaseType::Databend), Some("dbx_test"), "users"), "`dbx_test`.`users`"); assert_eq!( qualified_table_name(Some(DatabaseType::Xugu), Some("DBX_TEST"), "PRODUCTS"), diff --git a/crates/dbx-core/tests/mysql_cross_database_editing.rs b/crates/dbx-core/tests/mysql_cross_database_editing.rs new file mode 100644 index 0000000000..aea71ad430 --- /dev/null +++ b/crates/dbx-core/tests/mysql_cross_database_editing.rs @@ -0,0 +1,115 @@ +use dbx_core::data_grid_sql::{ + prepare_data_grid_save, DataGridColumnInfo, DataGridSaveStatementOptions, DataGridTableMeta, +}; +use dbx_core::db::ObjectSourceKind; +use dbx_core::models::connection::DatabaseType; +use dbx_core::schema::mysql_object_source_sql; +use dbx_core::sql_dialect::{ + build_count_table_sql, build_table_data_select_sql, build_table_select_sql, TableDataSelectSqlOptions, + TableSelectSqlOptions, +}; +use dbx_core::sql_editability::analyze_editable_query_editability; +use serde_json::json; + +#[test] +fn mysql_cross_database_query_flow_preserves_target_database() { + let analysis = analyze_editable_query_editability("SELECT * FROM db_9.users WHERE active = 1"); + assert!(analysis.editable, "simple cross-database mysql query should remain editable"); + let analysis = analysis.analysis.expect("editable query should include parsed analysis"); + assert_eq!(analysis.schema.as_deref(), Some("db_9")); + assert_eq!(analysis.table_name, "users"); + + let preview_sql = build_table_data_select_sql(TableDataSelectSqlOptions { + database_type: Some(DatabaseType::Mysql), + schema: Some("db_9".to_string()), + table_name: "users".to_string(), + table_type: Some("TABLE".to_string()), + primary_keys: vec!["id".to_string()], + columns: vec!["id".to_string(), "name".to_string()], + fallback_order_columns: vec![], + order_by: Some("`id` ASC".to_string()), + limit: Some(100), + offset: Some(0), + where_input: Some("active = 1".to_string()), + include_row_id: false, + }); + assert!(preview_sql.contains("FROM `db_9`.`users`"), "preview SQL should stay on the referenced database"); + + let export_sql = build_table_select_sql(TableSelectSqlOptions { + database_type: Some(DatabaseType::Mysql), + schema: Some("db_9"), + table_name: "users", + columns: &["id".to_string(), "name".to_string()], + order_columns: &["id".to_string()], + limit: 50, + }); + assert!( + export_sql.contains("FROM `db_9`.`users`"), + "table export/select SQL should stay on the referenced database" + ); + + let count_sql = build_count_table_sql(Some(DatabaseType::Mysql), Some("db_9"), "users"); + assert_eq!(count_sql, "SELECT COUNT(*) AS row_count FROM `db_9`.`users`"); + + let save = prepare_data_grid_save(DataGridSaveStatementOptions { + database_type: Some(DatabaseType::Mysql), + table_meta: DataGridTableMeta { + schema: Some("db_9".to_string()), + table_name: "users".to_string(), + primary_keys: vec!["id".to_string()], + columns: Some(vec![ + DataGridColumnInfo { + name: "id".to_string(), + data_type: "INT".to_string(), + is_nullable: false, + is_primary_key: true, + column_default: None, + extra: None, + }, + DataGridColumnInfo { + name: "name".to_string(), + data_type: "VARCHAR".to_string(), + is_nullable: true, + is_primary_key: false, + column_default: None, + extra: None, + }, + ]), + }, + columns: vec!["id".to_string(), "name".to_string()], + source_columns: None, + rows: vec![vec![json!(1), json!("before")]], + dirty_rows: vec![(0, vec![(1, json!("after"))])], + deleted_rows: vec![], + new_rows: vec![], + }); + + assert_eq!(save.execution_schema.as_deref(), Some("db_9")); + assert_eq!(save.statements.len(), 1); + assert!( + save.statements[0].contains("UPDATE `db_9`.`users` SET `name` = 'after'"), + "save SQL should update the referenced database: {}", + save.statements[0] + ); + assert!( + save.statements[0].contains("WHERE `id` = 1"), + "save SQL should still target the selected row by primary key: {}", + save.statements[0] + ); +} + +#[test] +fn mysql_cross_database_show_create_and_object_source_are_qualified() { + assert_eq!( + mysql_object_source_sql("db_9", "users_view", &ObjectSourceKind::View), + "SHOW CREATE VIEW `db_9`.`users_view`" + ); + assert_eq!( + mysql_object_source_sql("db_9", "sync_users", &ObjectSourceKind::Procedure), + "SHOW CREATE PROCEDURE `db_9`.`sync_users`" + ); + assert_eq!( + mysql_object_source_sql("db_9", "score_user", &ObjectSourceKind::Function), + "SHOW CREATE FUNCTION `db_9`.`score_user`" + ); +}