From 64ba6a537bbdf056f4c0f0ccd9778d237b097c80 Mon Sep 17 00:00:00 2001 From: zipg Date: Sat, 26 Sep 2026 01:30:34 +0800 Subject: [PATCH] fix(iris): paginate table data via %VID window and fix count quoting --- crates/dbx-sql-data/src/data_grid_sql.rs | 45 +++++++++++-------- .../src/sql_dialect/table_select.rs | 27 ++++++++++- .../dbx-sql-dialect/src/sql_dialect/tests.rs | 40 +++++++++++++++++ 3 files changed, 93 insertions(+), 19 deletions(-) diff --git a/crates/dbx-sql-data/src/data_grid_sql.rs b/crates/dbx-sql-data/src/data_grid_sql.rs index 1350e1cb32..c522716143 100644 --- a/crates/dbx-sql-data/src/data_grid_sql.rs +++ b/crates/dbx-sql-data/src/data_grid_sql.rs @@ -1060,25 +1060,17 @@ fn is_sqlserver_legacy_profile(driver_profile: Option<&str>) -> bool { } pub fn build_data_grid_count_sql(options: DataGridCountSqlOptions) -> String { - let table = if crate::sql_dialect::uses_connection_identifier_quote( + // Keep the reference identical to the one the grid's SELECT uses: Caché/IRIS + // reject quoted ordinary names when delimited identifiers are disabled, so + // the count must not be the only statement that quotes them (#8929). + let table = data_grid_qualified_table_name( options.database_type, + options.catalog.as_deref(), + options.schema.as_deref(), + options.database.as_deref(), + &options.table_name, options.identifier_quote.as_deref(), - ) { - crate::sql_dialect::table_data_qualified_table_name( - options.database_type, - options.schema.as_deref(), - &options.table_name, - options.identifier_quote.as_deref(), - ) - } else { - crate::sql_dialect::qualified_table_name_with_catalog( - options.database_type, - options.catalog.as_deref(), - options.schema.as_deref(), - options.database.as_deref(), - &options.table_name, - ) - }; + ); let predicate = crate::sql_dialect::normalize_where_input(options.where_input.as_deref()); let where_clause = if predicate.is_empty() { String::new() } else { format!(" WHERE ({predicate})") }; let hint = options.count_hint.as_deref().unwrap_or(""); @@ -3977,6 +3969,10 @@ mod tests { #[test] fn iris_data_grid_count_queries_the_table_without_wrapping_top_sql() { + // Caché 2016 runs with delimited identifiers disabled, where a quoted + // ordinary name is not a table reference — the count must use the same + // unquoted spelling as the grid SELECT (#8929). Delimited names keep + // their quotes. assert_eq!( build_data_grid_count_sql(DataGridCountSqlOptions { database_type: Some(DatabaseType::Iris), @@ -3988,7 +3984,20 @@ mod tests { where_input: Some("SSUSR_IsActive = 'Y'".to_string()), count_hint: None, }), - "SELECT COUNT(*) AS cnt FROM \"SS\".\"SS_User\" WHERE (SSUSR_IsActive = 'Y')" + "SELECT COUNT(*) AS cnt FROM SS.SS_User WHERE (SSUSR_IsActive = 'Y')" + ); + assert_eq!( + build_data_grid_count_sql(DataGridCountSqlOptions { + database_type: Some(DatabaseType::Iris), + identifier_quote: None, + catalog: None, + database: None, + schema: Some("App Schema".to_string()), + table_name: "Patient Record".to_string(), + where_input: None, + count_hint: None, + }), + "SELECT COUNT(*) AS cnt FROM \"App Schema\".\"Patient Record\"" ); } diff --git a/crates/dbx-sql-dialect/src/sql_dialect/table_select.rs b/crates/dbx-sql-dialect/src/sql_dialect/table_select.rs index 3ace9d1072..21d10a1593 100644 --- a/crates/dbx-sql-dialect/src/sql_dialect/table_select.rs +++ b/crates/dbx-sql-dialect/src/sql_dialect/table_select.rs @@ -335,7 +335,7 @@ pub fn build_table_data_select_sql_with_database( if options.use_driver_row_offset { format!("SELECT {select_columns} FROM {table_alias}{where_clause}{order}") } else { - format!("SELECT TOP {limit} {select_columns} FROM {table_alias}{where_clause}{order}") + build_iris_table_select_sql(&select_columns, &table_alias, &where_clause, &order, limit, offset) } } TablePaginationStrategy::InformixFirst => { @@ -670,6 +670,31 @@ fn quoted_table_columns_or_star(database_type: Option, columns: &[ .join(", ") } +/// Builds one page of a Caché/IRIS table read. +/// +/// InterSystems SQL has `TOP` but no `OFFSET` clause — both Caché 2016 and IRIS +/// reject `SELECT TOP n ... OFFSET m` — so later pages bound a derived table +/// with `TOP(offset + limit)` and drop the leading rows with `%VID`, the row +/// number InterSystems assigns to the rows a query produces. Running the same +/// `TOP limit` statement for every page was the reason the grid kept showing +/// the first page (#8929). +fn build_iris_table_select_sql( + select_columns: &str, + table_alias: &str, + where_clause: &str, + order: &str, + limit: usize, + offset: usize, +) -> String { + if offset == 0 { + return format!("SELECT TOP {limit} {select_columns} FROM {table_alias}{where_clause}{order}"); + } + let window = offset.saturating_add(limit); + format!( + "SELECT * FROM (SELECT TOP {window} {select_columns} FROM {table_alias}{where_clause}{order}) WHERE %VID > {offset}" + ) +} + fn build_rownum_table_select_sql( table: &str, where_clause: &str, diff --git a/crates/dbx-sql-dialect/src/sql_dialect/tests.rs b/crates/dbx-sql-dialect/src/sql_dialect/tests.rs index 7adb6d1fec..5633382341 100644 --- a/crates/dbx-sql-dialect/src/sql_dialect/tests.rs +++ b/crates/dbx-sql-dialect/src/sql_dialect/tests.rs @@ -1173,6 +1173,46 @@ fn builds_iris_table_data_sql_with_literal_top_and_ordinary_object() { assert!(!sql.contains(" LIMIT ")); } +#[test] +fn iris_table_data_pages_use_a_vid_window_instead_of_a_repeated_top() { + let options = TableDataSelectSqlOptions { + database_type: Some(DatabaseType::Iris), + schema: Some("Ens".to_string()), + table_name: "AlarmResponse".to_string(), + table_type: None, + primary_keys: vec!["ID".to_string()], + columns: vec!["ID".to_string(), "Status".to_string()], + fallback_order_columns: Vec::new(), + order_by: Some("ID ASC".to_string()), + limit: Some(25), + offset: Some(0), + where_input: Some("WHERE Status = 'Open'".to_string()), + include_row_id: false, + ..Default::default() + }; + // First page keeps the historical TOP-only statement. + assert_eq!( + build_table_data_select_sql(options.clone()), + "SELECT TOP 25 * FROM Ens.AlarmResponse WHERE (Status = 'Open') ORDER BY ID ASC" + ); + // Later pages must move the window: InterSystems SQL has no OFFSET clause, + // so the page is derived from TOP(offset + limit) and `%VID` drops the rows + // before the offset (#8929). + assert_eq!( + build_table_data_select_sql(TableDataSelectSqlOptions { offset: Some(100), ..options.clone() }), + "SELECT * FROM (SELECT TOP 125 * FROM Ens.AlarmResponse WHERE (Status = 'Open') ORDER BY ID ASC) WHERE %VID > 100" + ); + // The JDBC driver applies the offset itself, so its statement stays unwrapped. + assert_eq!( + build_table_data_select_sql(TableDataSelectSqlOptions { + offset: Some(100), + use_driver_row_offset: true, + ..options + }), + "SELECT * FROM Ens.AlarmResponse WHERE (Status = 'Open') ORDER BY ID ASC" + ); +} + #[test] fn iris_table_data_sql_quotes_only_delimited_object_names() { let sql = build_table_data_select_sql(TableDataSelectSqlOptions {