From 8747a0e8f964b2259d64dc7b12956edbf07507c8 Mon Sep 17 00:00:00 2001 From: Guoyu Su Date: Tue, 30 Jun 2026 19:51:03 +0800 Subject: [PATCH] fix(clickhouse): place pagination before settings clause (#2206) --- crates/dbx-core/src/query_result_sql.rs | 57 ++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/crates/dbx-core/src/query_result_sql.rs b/crates/dbx-core/src/query_result_sql.rs index 10545d501..d6655bb90 100644 --- a/crates/dbx-core/src/query_result_sql.rs +++ b/crates/dbx-core/src/query_result_sql.rs @@ -862,7 +862,11 @@ fn add_standard_limit( return format!("{statement};"); } let offset_sql = if offset > 0 { format!(" OFFSET {offset}") } else { String::new() }; - append_sql_suffix(statement, &format!("{order_sql} LIMIT {limit}{offset_sql};")) + let limit_sql = format!("{order_sql} LIMIT {limit}{offset_sql}"); + if database_type == Some(DatabaseType::ClickHouse) { + return add_clickhouse_limit(statement, &limit_sql); + } + append_sql_suffix(statement, &format!("{limit_sql};")) } fn add_outer_standard_limit( @@ -876,6 +880,20 @@ fn add_outer_standard_limit( derived_table_sql("SELECT * FROM", statement, &format!("{alias}{order_sql} LIMIT {limit} OFFSET {offset};")) } +fn add_clickhouse_limit(statement: &str, limit_sql: &str) -> String { + let limit_sql = limit_sql.trim(); + let settings_index = + top_level_sql_tokens(statement).iter().find(|token| token.text == "SETTINGS").map(|token| token.start); + + if let Some(index) = settings_index { + let statement_before_settings = statement[..index].trim_end(); + let settings_clause = statement[index..].trim_start(); + return format!("{statement_before_settings} {limit_sql} {settings_clause};"); + } + + append_sql_suffix(statement, &format!("{limit_sql};")) +} + fn derived_table_sql(prefix: &str, statement: &str, suffix: &str) -> String { format!("{prefix} ({}) {suffix}", statement_for_sql_suffix(statement)) } @@ -1510,6 +1528,43 @@ WHERE u.id = picked.id; ); } + #[test] + fn clickhouse_settings_clause_is_paginated_before_settings() { + let result = build_paginated_query_sql(PaginatedQuerySqlOptions { + original_sql: "SELECT * FROM system.clusters SETTINGS max_execution_time = 0".to_string(), + database_type: Some(DatabaseType::ClickHouse), + limit: 50, + offset: 100, + }); + + assert!(result.ok); + assert_eq!( + result.sql.unwrap(), + "SELECT * FROM system.clusters LIMIT 50 OFFSET 100 SETTINGS max_execution_time = 0;" + ); + } + + #[test] + fn clickhouse_query_plan_places_limit_before_settings() { + let sql = "SELECT * FROM system.clusters SETTINGS max_execution_time = 0"; + let plan = build_query_pagination_execution_plan(QueryPaginationExecutionPlanOptions { + sql: sql.to_string(), + query_base_sql: sql.to_string(), + database_type: Some(DatabaseType::ClickHouse), + pagination: QueryPagination { limit: 100, offset: 0, session_id: None }, + use_agent_cursor: false, + first_page_uses_actual_sql: false, + }); + + assert_eq!(plan.sql_to_execute, "SELECT * FROM system.clusters LIMIT 100 SETTINGS max_execution_time = 0;"); + assert_eq!( + plan.page_sql, + Some("SELECT * FROM system.clusters LIMIT 100 SETTINGS max_execution_time = 0;".to_string()) + ); + assert_eq!(plan.page_limit, Some(100)); + assert_eq!(plan.page_offset, Some(0)); + } + #[test] fn clickhouse_scalar_with_select_can_be_counted() { let result = build_count_query_sql(CountQuerySqlOptions {