diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index e8000030..651487de 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -166,6 +166,15 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "ar_archive_writer" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7eb93bbb63b9c227414f6eb3a0adfddca591a8ce1e9b60661bb08969b87e340b" +dependencies = [ + "object", +] + [[package]] name = "arbitrary" version = "1.4.2" @@ -3846,6 +3855,15 @@ dependencies = [ "objc2-security", ] +[[package]] +name = "object" +version = "0.37.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ff76201f031d8863c38aa7f905eca4f53abbfa15f609db4277d44cd8938f33fe" +dependencies = [ + "memchr", +] + [[package]] name = "once_cell" version = "1.21.3" @@ -4575,6 +4593,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "psm" +version = "0.1.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "645dbe486e346d9b5de3ef16ede18c26e6c70ad97418f4874b8b1889d6e761ea" +dependencies = [ + "ar_archive_writer", + "cc", +] + [[package]] name = "ptr_meta" version = "0.1.4" @@ -4835,6 +4863,26 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "recursive" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0786a43debb760f491b1bc0269fe5e84155353c67482b9e60d0cfb596054b43e" +dependencies = [ + "recursive-proc-macro-impl", + "stacker", +] + +[[package]] +name = "recursive-proc-macro-impl" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76009fbe0614077fc1a2ce255e3a1881a2e3a3527097d5dc6d8212c585e7e38b" +dependencies = [ + "quote", + "syn 2.0.117", +] + [[package]] name = "redox_syscall" version = "0.5.18" @@ -5825,6 +5873,16 @@ dependencies = [ "der", ] +[[package]] +name = "sqlparser" +version = "0.62.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13c6d1b651dc4edf07eead2a0c6c78016ce971bc2c10da5266861b13f25e7cec" +dependencies = [ + "log", + "recursive", +] + [[package]] name = "sqlx" version = "0.8.6" @@ -6031,6 +6089,19 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" +[[package]] +name = "stacker" +version = "0.1.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "640c8cdd92b6b12f5bcb1803ca3bbf5ab96e5e6b6b96b9ab77dabe9e880b3190" +dependencies = [ + "cc", + "cfg-if", + "libc", + "psm", + "windows-sys 0.61.2", +] + [[package]] name = "static_assertions" version = "1.1.0" @@ -6188,7 +6259,7 @@ dependencies = [ [[package]] name = "tabularis" -version = "0.12.0" +version = "0.13.0" dependencies = [ "async-trait", "base64 0.22.1", @@ -6218,6 +6289,7 @@ dependencies = [ "serde_json", "serde_yaml", "sha2", + "sqlparser", "sqlx", "sysinfo", "tauri", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 90524788..d8b42007 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -68,6 +68,7 @@ rustls-pemfile = "2" rustls-platform-verifier = "0.6" notify = "6" ulid = "1.2.1" +sqlparser = "0.62" # GTK dependencies for Wayland window title workaround (Linux only) [target.'cfg(target_os = "linux")'.dependencies] diff --git a/src-tauri/src/drivers/common.rs b/src-tauri/src/drivers/common.rs index 281659fb..d238b965 100644 --- a/src-tauri/src/drivers/common.rs +++ b/src-tauri/src/drivers/common.rs @@ -10,9 +10,9 @@ pub use blob::{ DEFAULT_MAX_BLOB_SIZE, MAX_BLOB_PREVIEW_SIZE, }; pub use query::{ - build_paginated_query, calculate_offset, extract_user_limit, extract_user_offset, - is_explainable_query, - is_select_query, returns_result_set, strip_leading_sql_comments, strip_limit_offset, + annotate_error_with_query, build_paginated_query, calculate_offset, extract_user_limit, + extract_user_offset, is_explainable_query, is_select_query, returns_result_set, + strip_leading_sql_comments, strip_limit_offset, PaginationDialect, EXECUTED_QUERY_MARKER, }; pub use safe_int::{ i64_to_json, parse_unsafe_bigint_string, u64_to_json, JS_MAX_SAFE_INTEGER, JS_MAX_SAFE_UINT, diff --git a/src-tauri/src/drivers/common/query.rs b/src-tauri/src/drivers/common/query.rs index 4d09e4ca..4590c97f 100644 --- a/src-tauri/src/drivers/common/query.rs +++ b/src-tauri/src/drivers/common/query.rs @@ -1,3 +1,7 @@ +use sqlparser::ast::{Expr, LimitClause, Offset, OffsetRows, Statement, Value}; +use sqlparser::dialect::{Dialect, MySqlDialect, PostgreSqlDialect, SQLiteDialect}; +use sqlparser::parser::Parser; + /// Check if a query is a SELECT statement. /// /// Leading SQL comments are stripped before checking, matching @@ -281,22 +285,182 @@ pub fn extract_user_offset(query: &str) -> Option { None } +/// SQL dialect used to parse a query before rewriting its pagination. +/// +/// Driver-facing wrapper so the `sqlparser` dialect types stay internal to +/// this module — each driver passes the variant matching its engine. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PaginationDialect { + MySql, + Postgres, + Sqlite, +} + +impl PaginationDialect { + fn parser_dialect(self) -> Box { + match self { + PaginationDialect::MySql => Box::new(MySqlDialect {}), + PaginationDialect::Postgres => Box::new(PostgreSqlDialect {}), + PaginationDialect::Sqlite => Box::new(SQLiteDialect {}), + } + } +} + +/// LIMIT/OFFSET read from a query's parsed AST. +struct ParsedPagination { + /// User-supplied LIMIT, if it is a plain numeric literal. + user_limit: Option, + /// User-supplied OFFSET (0 when absent or non-numeric). + user_offset: u32, + /// Whether the query carries a top-level LIMIT/OFFSET clause to strip. + has_limit_clause: bool, +} + +/// Resolve a numeric-literal expression to a `u32`. +/// +/// Non-literal expressions (placeholders, arithmetic, bind parameters) yield +/// `None` so they are treated as "no value" rather than being mis-read. +fn eval_u32(expr: &Expr) -> Option { + match expr { + Expr::Value(value) => match &value.value { + Value::Number(n, _) => n.parse().ok(), + _ => None, + }, + _ => None, + } +} + +/// Parse `query` with `dialect` and read the top-level LIMIT/OFFSET from the +/// AST, normalising MySQL's `LIMIT , ` form to the same shape. +/// +/// Returns `None` — so the caller falls back to the token heuristics — when +/// the input is not a single query the parser understands, or when it relies +/// on a `FETCH FIRST … ROWS` clause this rewriter does not handle. +fn parse_pagination(query: &str, dialect: PaginationDialect) -> Option { + let statements = Parser::parse_sql(dialect.parser_dialect().as_ref(), query).ok()?; + if statements.len() != 1 { + return None; + } + let Statement::Query(q) = &statements[0] else { + return None; + }; + + // FETCH FIRST … ROWS ONLY is out of scope; defer to the fallback path so + // its behaviour is unchanged rather than producing a mixed clause. + if q.fetch.is_some() { + return None; + } + + match &q.limit_clause { + None => Some(ParsedPagination { + user_limit: None, + user_offset: 0, + has_limit_clause: false, + }), + Some(LimitClause::LimitOffset { limit, offset, .. }) => Some(ParsedPagination { + user_limit: limit.as_ref().and_then(eval_u32), + user_offset: offset + .as_ref() + .and_then(|o| eval_u32(&o.value)) + .unwrap_or(0), + has_limit_clause: true, + }), + Some(LimitClause::OffsetCommaLimit { offset, limit }) => Some(ParsedPagination { + user_limit: eval_u32(limit), + user_offset: eval_u32(offset).unwrap_or(0), + has_limit_clause: true, + }), + } +} + +/// True for the keyword/value tokens that make up a trailing LIMIT/OFFSET +/// clause (standard, MySQL comma, and FETCH spellings), so a LIMIT/OFFSET that +/// appears mid-query is never mistaken for the trailing pagination clause. +/// +/// The numeric arm accepts digits interleaved with commas so MySQL's +/// `LIMIT ,` is recognised even when written without spaces +/// (`LIMIT 0,1`): the tokenizer splits only on whitespace, so the count and +/// offset arrive glued together as a single `0,1` token. +fn is_pagination_tail_token(tok: &str) -> bool { + let upper = tok.to_uppercase(); + matches!( + upper.as_str(), + "LIMIT" | "OFFSET" | "BY" | "ROW" | "ROWS" | "ONLY" | "NEXT" | "FIRST" | "," + ) || (!tok.is_empty() && tok.chars().all(|c| c.is_ascii_digit() || c == ',')) +} + +/// Cut the query immediately before its trailing top-level LIMIT/OFFSET clause. +/// +/// Reuses [`tokenize_sql_with_pos`], which collapses parenthesised groups into +/// a single token, so a LIMIT/OFFSET inside a subquery is never seen and only +/// the outer clause is removed. Unlike [`strip_limit_offset`], it cuts at the +/// keyword regardless of the value shape, so MySQL's `LIMIT , ` +/// is handled. Falls back to [`strip_limit_offset`] if no trailing clause is +/// found (kept total; the parser having reported a clause makes this rare). +fn strip_at_limit_keyword(query: &str) -> String { + let trimmed = query.trim_end(); + let tokens = tokenize_sql_with_pos(trimmed); + for (idx, (tok, pos)) in tokens.iter().enumerate() { + let upper = tok.to_uppercase(); + if (upper == "LIMIT" || upper == "OFFSET") + && tokens[idx + 1..] + .iter() + .all(|(t, _)| is_pagination_tail_token(t)) + { + return trimmed[..*pos].trim_end().to_string(); + } + } + strip_limit_offset(query) +} + +/// Build a numeric-literal expression for the rendered pagination clause. +fn number_expr(n: u32) -> Expr { + Expr::value(Value::Number(n.to_string(), false)) +} + /// Build a paginated query by stripping any user-supplied LIMIT/OFFSET and /// appending pagination clauses directly. ORDER BY is left in place so that /// table-qualified column references (e.g. `o.created_at`) remain valid — /// wrapping the original query in a subquery would move those references out /// of scope and cause "unknown column" errors. /// +/// The user's LIMIT/OFFSET are read from the parsed AST (using `dialect`), so +/// dialect-specific forms such as MySQL's `LIMIT , ` and +/// `OFFSET` before `LIMIT` are understood. When the parser cannot handle the +/// input, it falls back to a token-aware heuristic scan. The appended +/// pagination clause is rendered from a [`LimitClause`] AST node and +/// concatenated to the verbatim sliced base, so leading comments, inline +/// hints, and the body's formatting are preserved. +/// /// When the user wrote an explicit LIMIT, it is honoured as a cap on the total /// number of rows returned across all pages. A user-supplied OFFSET is honoured /// too: it is added to the per-page offset so that pagination walks the result /// set the user actually asked for (the rows after their OFFSET). Discarding it /// silently collapsed `LIMIT 1 OFFSET 1` to `LIMIT 1 OFFSET 0` on page 1. -pub fn build_paginated_query(query: &str, page_size: u32, page: u32) -> String { +pub fn build_paginated_query( + query: &str, + page_size: u32, + page: u32, + dialect: PaginationDialect, +) -> String { let page_offset = calculate_offset(page, page_size); - let user_limit = extract_user_limit(query); - let user_offset = extract_user_offset(query).unwrap_or(0); - let base = strip_limit_offset(query); + + let (user_limit, user_offset, base) = match parse_pagination(query, dialect) { + Some(parsed) => { + let base = if parsed.has_limit_clause { + strip_at_limit_keyword(query) + } else { + query.trim_end().to_string() + }; + (parsed.user_limit, parsed.user_offset, base) + } + // Parser could not handle the input — fall back to the token heuristics. + None => ( + extract_user_limit(query), + extract_user_offset(query).unwrap_or(0), + strip_limit_offset(query), + ), + }; let fetch_count = match user_limit { Some(ul) => { @@ -309,5 +473,43 @@ pub fn build_paginated_query(query: &str, page_size: u32, page: u32) -> String { let offset = user_offset.saturating_add(page_offset); - format!("{} LIMIT {} OFFSET {}", base, fetch_count, offset) + // Render the pagination clause from an AST node so the output is built by + // the parser rather than hand-formatted. + let clause = LimitClause::LimitOffset { + limit: Some(number_expr(fetch_count)), + offset: Some(Offset { + value: number_expr(offset), + rows: OffsetRows::None, + }), + limit_by: Vec::new(), + }; + + // `LimitClause`'s Display renders a leading space (it is meant to follow a + // preceding clause), so concatenate without inserting another one. + format!("{}{}", base, clause) +} + +/// Sentinel that separates a human error message from the actual SQL that +/// produced it inside a single error string. The leading code point is from +/// the Unicode private-use area, so it never appears in real SQL text or DB +/// driver error messages and survives JSON/IPC transport unescaped — letting +/// the frontend split on this marker without colliding with the `\n\n` +/// brief/detail convention. Must stay in sync with the parser in +/// `src/components/ui/ErrorDisplay.tsx`. +pub const EXECUTED_QUERY_MARKER: &str = "\u{E000}__TABULARIS_EXECUTED_QUERY__"; + +/// Appends the executed SQL to a DB error message so the UI can show the user +/// the exact statement that failed. This matters when pagination rewrote the +/// query (appending `LIMIT`/`OFFSET`): the database complains about clauses the +/// user never typed, and without the rewritten text the error is baffling. +/// +/// When `executed` matches `original` (ignoring surrounding whitespace) the +/// error is returned unchanged — echoing back the query the user can already +/// see would only add noise. +pub fn annotate_error_with_query(err: String, executed: &str, original: &str) -> String { + if executed.trim() == original.trim() { + err + } else { + format!("{err}{EXECUTED_QUERY_MARKER}{executed}") + } } diff --git a/src-tauri/src/drivers/common/tests.rs b/src-tauri/src/drivers/common/tests.rs index 9fd46ec2..0c604548 100644 --- a/src-tauri/src/drivers/common/tests.rs +++ b/src-tauri/src/drivers/common/tests.rs @@ -1,10 +1,35 @@ use super::{ - build_paginated_query, decode_blob_wire_format, encode_blob, encode_blob_full, i64_to_json, - is_explainable_query, is_select_query, parse_unsafe_bigint_string, strip_leading_sql_comments, - strip_limit_offset, u64_to_json, DEFAULT_MAX_BLOB_SIZE, JS_MAX_SAFE_INTEGER, JS_MAX_SAFE_UINT, + annotate_error_with_query, build_paginated_query, decode_blob_wire_format, encode_blob, + encode_blob_full, i64_to_json, is_explainable_query, is_select_query, parse_unsafe_bigint_string, + strip_leading_sql_comments, strip_limit_offset, u64_to_json, PaginationDialect, + DEFAULT_MAX_BLOB_SIZE, EXECUTED_QUERY_MARKER, JS_MAX_SAFE_INTEGER, JS_MAX_SAFE_UINT, MAX_BLOB_PREVIEW_SIZE, }; +#[test] +fn test_annotate_error_appends_rewritten_query() { + let err = "syntax error near 'LIMIT 1 OFFSET 0'".to_string(); + let executed = "SELECT * FROM t LIMIT 1 OFFSET 0"; + let original = "SELECT * FROM t"; + let out = annotate_error_with_query(err.clone(), executed, original); + + let (message, query) = out + .split_once(EXECUTED_QUERY_MARKER) + .expect("marker should be present when the query was rewritten"); + assert_eq!(message, err); + assert_eq!(query, executed); +} + +#[test] +fn test_annotate_error_noop_when_query_unchanged() { + let err = "permission denied".to_string(); + let q = "SELECT * FROM t"; + // Identical query (modulo surrounding whitespace) must not be appended. + let out = annotate_error_with_query(err.clone(), " SELECT * FROM t ", q); + assert_eq!(out, err); + assert!(!out.contains(EXECUTED_QUERY_MARKER)); +} + #[test] fn test_decode_blob_wire_format_valid() { // Encode some known bytes, then verify decode round-trips correctly @@ -277,7 +302,7 @@ fn test_extract_user_limit_table_name_contains_limit_with_real_limit() { #[test] fn test_build_paginated_query_no_user_limit() { let q = "SELECT o.id FROM orders o ORDER BY o.created_at DESC"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "SELECT o.id FROM orders o ORDER BY o.created_at DESC LIMIT 101 OFFSET 0" @@ -287,7 +312,7 @@ fn test_build_paginated_query_no_user_limit() { #[test] fn test_build_paginated_query_replaces_user_limit() { let q = "SELECT * FROM t ORDER BY id LIMIT 50"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); // User wanted 50 rows. page_size=100, so remaining=50, fetch = min(50, 101) = 50 assert_eq!(result, "SELECT * FROM t ORDER BY id LIMIT 50 OFFSET 0"); } @@ -295,7 +320,7 @@ fn test_build_paginated_query_replaces_user_limit() { #[test] fn test_build_paginated_query_user_limit_second_page() { let q = "SELECT * FROM t ORDER BY id LIMIT 250"; - let result = build_paginated_query(q, 100, 2); + let result = build_paginated_query(q, 100, 2, PaginationDialect::Postgres); // offset=100, remaining=150, fetch = min(150, 101) = 101 assert_eq!(result, "SELECT * FROM t ORDER BY id LIMIT 101 OFFSET 100"); } @@ -303,7 +328,7 @@ fn test_build_paginated_query_user_limit_second_page() { #[test] fn test_build_paginated_query_user_limit_exhausted() { let q = "SELECT * FROM t LIMIT 50"; - let result = build_paginated_query(q, 100, 2); + let result = build_paginated_query(q, 100, 2, PaginationDialect::Postgres); // offset=100, remaining=0 (50-100 saturates to 0), fetch = min(0, 101) = 0 assert_eq!(result, "SELECT * FROM t LIMIT 0 OFFSET 100"); } @@ -311,7 +336,7 @@ fn test_build_paginated_query_user_limit_exhausted() { #[test] fn test_build_paginated_query_table_name_contains_limit() { let q = "SELECT * FROM tapp_appointment_message_event_limit ORDER BY id"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "SELECT * FROM tapp_appointment_message_event_limit ORDER BY id LIMIT 101 OFFSET 0" @@ -321,7 +346,7 @@ fn test_build_paginated_query_table_name_contains_limit() { #[test] fn test_build_paginated_query_table_name_contains_limit_with_user_limit() { let q = "SELECT * FROM tapp_appointment_message_event_limit ORDER BY id LIMIT 10"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "SELECT * FROM tapp_appointment_message_event_limit ORDER BY id LIMIT 10 OFFSET 0" @@ -357,7 +382,7 @@ fn test_build_paginated_query_preserves_user_offset() { // Regression for #273: `LIMIT 1 OFFSET 1` must keep OFFSET 1 on page 1, // not collapse to OFFSET 0 (which returned the 1st row instead of the 2nd). let q = "SELECT DISTINCT salary FROM employees ORDER BY salary DESC LIMIT 1 OFFSET 1"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "SELECT DISTINCT salary FROM employees ORDER BY salary DESC LIMIT 1 OFFSET 1" @@ -367,7 +392,7 @@ fn test_build_paginated_query_preserves_user_offset() { #[test] fn test_build_paginated_query_user_offset_no_limit() { let q = "SELECT * FROM t ORDER BY id OFFSET 5"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!(result, "SELECT * FROM t ORDER BY id LIMIT 101 OFFSET 5"); } @@ -375,14 +400,14 @@ fn test_build_paginated_query_user_offset_no_limit() { fn test_build_paginated_query_user_offset_second_page() { // page offset (100) is added on top of the user's OFFSET (5). let q = "SELECT * FROM t ORDER BY id OFFSET 5"; - let result = build_paginated_query(q, 100, 2); + let result = build_paginated_query(q, 100, 2, PaginationDialect::Postgres); assert_eq!(result, "SELECT * FROM t ORDER BY id LIMIT 101 OFFSET 105"); } #[test] fn test_build_paginated_query_subquery_with_limit() { let q = "SELECT * FROM (SELECT id FROM t ORDER BY id LIMIT 100) sub ORDER BY id LIMIT 5"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "SELECT * FROM (SELECT id FROM t ORDER BY id LIMIT 100) sub ORDER BY id LIMIT 5 OFFSET 0" @@ -392,7 +417,7 @@ fn test_build_paginated_query_subquery_with_limit() { #[test] fn test_build_paginated_query_with_leading_comments() { let q = "-- header\nSELECT * FROM t ORDER BY id"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "-- header\nSELECT * FROM t ORDER BY id LIMIT 101 OFFSET 0" @@ -405,13 +430,81 @@ fn test_build_paginated_query_multiline_comments_with_user_limit() { // appended `LIMIT … OFFSET …` lands on its own line rather than // being swallowed into the `--` header as comment text. let q = "-- ============\n-- title\n-- ============\n\nSELECT * FROM t ORDER BY id LIMIT 50"; - let result = build_paginated_query(q, 100, 1); + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); assert_eq!( result, "-- ============\n-- title\n-- ============\n\nSELECT * FROM t ORDER BY id LIMIT 50 OFFSET 0" ); } +#[test] +fn test_build_paginated_query_mysql_comma_limit() { + // MySQL `LIMIT , ` — the parser normalises it so the offset + // (10) folds into the page offset and the count (5) caps the fetch. + let q = "SELECT * FROM t LIMIT 10, 5"; + let result = build_paginated_query(q, 100, 1, PaginationDialect::MySql); + assert_eq!(result, "SELECT * FROM t LIMIT 5 OFFSET 10"); +} + +#[test] +fn test_build_paginated_query_mysql_comma_limit_no_space() { + // No space between offset and count (`LIMIT 0,1`). The whitespace tokenizer + // keeps `0,1` as a single token, so the trailing clause must still be + // recognised and stripped rather than producing the invalid + // `... LIMIT 0,1 LIMIT 1 OFFSET 0`. + let q = "select * from accounts LIMIT 0,1"; + let result = build_paginated_query(q, 100, 1, PaginationDialect::MySql); + assert_eq!(result, "select * from accounts LIMIT 1 OFFSET 0"); +} + +#[test] +fn test_build_paginated_query_mysql_comma_limit_second_page() { + let q = "SELECT * FROM t LIMIT 10, 250"; + let result = build_paginated_query(q, 100, 2, PaginationDialect::MySql); + // user_offset=10, user_limit=250, page_offset=100. + // fetch = min(250-100, 101) = 101, offset = 10 + 100 = 110 + assert_eq!(result, "SELECT * FROM t LIMIT 101 OFFSET 110"); +} + +#[test] +fn test_build_paginated_query_offset_before_limit() { + // Postgres allows OFFSET before LIMIT; both must be stripped and folded. + let q = "SELECT * FROM t ORDER BY id OFFSET 5 LIMIT 10"; + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); + assert_eq!(result, "SELECT * FROM t ORDER BY id LIMIT 10 OFFSET 5"); +} + +#[test] +fn test_build_paginated_query_mysql_backtick_identifier() { + let q = "SELECT * FROM `orders` ORDER BY `id` LIMIT 10"; + let result = build_paginated_query(q, 100, 1, PaginationDialect::MySql); + assert_eq!( + result, + "SELECT * FROM `orders` ORDER BY `id` LIMIT 10 OFFSET 0" + ); +} + +#[test] +fn test_build_paginated_query_preserves_inline_hint() { + // The body is sliced verbatim, so an inline comment/optimizer hint is kept + // even though the parser itself discards comments. + let q = "SELECT /*+ MAX_EXECUTION_TIME(1000) */ * FROM t LIMIT 5"; + let result = build_paginated_query(q, 100, 1, PaginationDialect::MySql); + assert_eq!( + result, + "SELECT /*+ MAX_EXECUTION_TIME(1000) */ * FROM t LIMIT 5 OFFSET 0" + ); +} + +#[test] +fn test_build_paginated_query_falls_back_on_parse_error() { + // The parser rejects this (dangling LIMIT after WHERE), so the rewriter + // falls back to the token heuristics and still strips the trailing LIMIT. + let q = "SELECT * FROM t WHERE LIMIT 5"; + let result = build_paginated_query(q, 100, 1, PaginationDialect::Postgres); + assert_eq!(result, "SELECT * FROM t WHERE LIMIT 5 OFFSET 0"); +} + #[test] fn test_strip_limit_offset_preserves_leading_comments() { assert_eq!( diff --git a/src-tauri/src/drivers/mysql/mod.rs b/src-tauri/src/drivers/mysql/mod.rs index bd2ce962..bbe953b5 100644 --- a/src-tauri/src/drivers/mysql/mod.rs +++ b/src-tauri/src/drivers/mysql/mod.rs @@ -948,7 +948,12 @@ async fn exec_on_mysql_conn( if is_select && limit.is_some() { let l = limit.unwrap(); - final_query = crate::drivers::common::build_paginated_query(query, l, page); + final_query = crate::drivers::common::build_paginated_query( + query, + l, + page, + crate::drivers::common::PaginationDialect::MySql, + ); pagination = Some(Pagination { page, @@ -994,7 +999,13 @@ async fn exec_on_mysql_conn( } json_rows.push(json_row); } - Err(e) => return Err(e.to_string()), + Err(e) => { + return Err(crate::drivers::common::annotate_error_with_query( + e.to_string(), + &final_query, + query, + )) + } } } } // rows_stream dropped here — conn borrow released diff --git a/src-tauri/src/drivers/postgres/mod.rs b/src-tauri/src/drivers/postgres/mod.rs index 410432af..55ea9271 100644 --- a/src-tauri/src/drivers/postgres/mod.rs +++ b/src-tauri/src/drivers/postgres/mod.rs @@ -810,7 +810,12 @@ async fn exec_on_pg_client( let (final_query, pagination_meta) = if is_select && limit.is_some() { let l = limit.unwrap(); - let data_query = crate::drivers::common::build_paginated_query(query, l, page); + let data_query = crate::drivers::common::build_paginated_query( + query, + l, + page, + crate::drivers::common::PaginationDialect::Postgres, + ); manual_limit = None; (data_query, Some((l, page))) } else { @@ -822,7 +827,11 @@ async fn exec_on_pg_client( client .query_raw(&final_query, &pg_params) .await - .map_err(|e| format_pg_error(&e))? + .map_err(|e| crate::drivers::common::annotate_error_with_query( + format_pg_error(&e), + &final_query, + query + ))? ); let mut columns: Vec = Vec::new(); @@ -851,7 +860,13 @@ async fn exec_on_pg_client( } json_rows.push(json_row); } - Err(e) => return Err(format_pg_error(&e)), + Err(e) => { + return Err(crate::drivers::common::annotate_error_with_query( + format_pg_error(&e), + &final_query, + query, + )) + } } } diff --git a/src-tauri/src/drivers/sqlite/mod.rs b/src-tauri/src/drivers/sqlite/mod.rs index 531ddedc..f747b896 100644 --- a/src-tauri/src/drivers/sqlite/mod.rs +++ b/src-tauri/src/drivers/sqlite/mod.rs @@ -540,7 +540,12 @@ async fn exec_on_sqlite_conn( if is_select && limit.is_some() { let l = limit.unwrap(); - final_query = crate::drivers::common::build_paginated_query(query, l, page); + final_query = crate::drivers::common::build_paginated_query( + query, + l, + page, + crate::drivers::common::PaginationDialect::Sqlite, + ); pagination = Some(Pagination { page, @@ -584,7 +589,13 @@ async fn exec_on_sqlite_conn( } json_rows.push(json_row); } - Err(e) => return Err(e.to_string()), + Err(e) => { + return Err(crate::drivers::common::annotate_error_with_query( + e.to_string(), + &final_query, + query, + )) + } } } diff --git a/src/components/notebook/SqlCell.tsx b/src/components/notebook/SqlCell.tsx index 690fa5e1..e08bdd34 100644 --- a/src/components/notebook/SqlCell.tsx +++ b/src/components/notebook/SqlCell.tsx @@ -35,6 +35,7 @@ export function SqlCell({ - + ); } diff --git a/src/components/ui/ErrorDisplay.tsx b/src/components/ui/ErrorDisplay.tsx index a60160c1..5f350cab 100644 --- a/src/components/ui/ErrorDisplay.tsx +++ b/src/components/ui/ErrorDisplay.tsx @@ -5,37 +5,104 @@ import type { TFunction } from "i18next"; interface ErrorDisplayProps { error: string; t: TFunction; + /** The query the user submitted, shown in a collapsible block so they can + * see exactly what was run when an error occurs. */ + originalQuery?: string; } -export function ErrorDisplay({ error, t }: ErrorDisplayProps) { - const [showDetails, setShowDetails] = useState(false); +// Sentinel used by the Rust drivers to attach the actually-executed SQL to an +// error string. Kept in sync with `EXECUTED_QUERY_MARKER` in +// `src-tauri/src/drivers/common/query.rs` (a Unicode private-use code point +// that never appears in real SQL or error text). +const EXECUTED_QUERY_MARKER = "\uE000__TABULARIS_EXECUTED_QUERY__"; - const separatorIndex = error.indexOf("\n\n"); - const hasDetails = separatorIndex !== -1 && separatorIndex < error.length - 2; - const brief = hasDetails ? error.slice(0, separatorIndex) : error; - const details = hasDetails ? error.slice(separatorIndex + 2) : ""; +/** A chevron-toggled block: a small button that reveals a SQL/details panel. */ +function Collapsible({ + showLabel, + hideLabel, + children, + variant = "details", +}: { + showLabel: string; + hideLabel: string; + children: React.ReactNode; + variant?: "details" | "query"; +}) { + const [open, setOpen] = useState(false); + return ( + <> + + {open && + (variant === "query" ? ( + 
+            {children}
+
+ ) : ( +
+ {children} +
+ ))} + + ); +} + +export function ErrorDisplay({ error, t, originalQuery }: ErrorDisplayProps) { + // Peel off the executed-query block (if the driver attached one) before + // applying the brief/detail split, so the SQL never leaks into the details. + const markerIndex = error.indexOf(EXECUTED_QUERY_MARKER); + const message = markerIndex === -1 ? error : error.slice(0, markerIndex); + const executedQuery = + markerIndex === -1 + ? "" + : error.slice(markerIndex + EXECUTED_QUERY_MARKER.length); + + const separatorIndex = message.indexOf("\n\n"); + const hasDetails = + separatorIndex !== -1 && separatorIndex < message.length - 2; + const brief = hasDetails ? message.slice(0, separatorIndex) : message; + const details = hasDetails ? message.slice(separatorIndex + 2) : ""; + + const original = originalQuery?.trim() ?? ""; + // Only surface the executed query when pagination actually rewrote the input + // — otherwise it just duplicates the original query the user already sees. + const showExecuted = + executedQuery.trim().length > 0 && executedQuery.trim() !== original; return (
Error: {brief}
{hasDetails && ( - <> - - {showDetails && ( -
- {details} -
- )} - + + {details} + + )} + {original && ( + + {original} + + )} + {showExecuted && ( + + {executedQuery} + )}
); diff --git a/src/components/ui/ResultEntryContent.tsx b/src/components/ui/ResultEntryContent.tsx index 7338fe5a..320e718b 100644 --- a/src/components/ui/ResultEntryContent.tsx +++ b/src/components/ui/ResultEntryContent.tsx @@ -45,7 +45,7 @@ export function ResultEntryContent({ if (entry.error) { return (
- +
); } diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index af303df5..7967837f 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -815,6 +815,10 @@ "queryFailed": "Abfrage fehlgeschlagen.", "showErrorDetails": "Details anzeigen", "hideErrorDetails": "Details ausblenden", + "showQuery": "Abfrage anzeigen", + "hideQuery": "Abfrage ausblenden", + "showExecutedQuery": "Ausgeführte Abfrage anzeigen", + "hideExecutedQuery": "Ausgeführte Abfrage ausblenden", "errorBoundary": { "title": "Der Editor ist unerwartet abgestürzt", "description": "Etwas im Editor konnte nicht gerendert werden. Der Fehler wird unten angezeigt — versuche es erneut oder kehre zu deinen Verbindungen zurück.", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 0d9ba028..3c21e313 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -816,6 +816,10 @@ "queryFailed": "Query failed.", "showErrorDetails": "Show details", "hideErrorDetails": "Hide details", + "showQuery": "Show query", + "hideQuery": "Hide query", + "showExecutedQuery": "Show executed query", + "hideExecutedQuery": "Hide executed query", "errorBoundary": { "title": "The editor crashed unexpectedly", "description": "Something in the editor failed to render. The error is shown below — try again, or go back to your connections.", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index 9eb9f739..1250c0f4 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -813,6 +813,10 @@ "queryFailed": "La consulta falló.", "showErrorDetails": "Mostrar detalles", "hideErrorDetails": "Ocultar detalles", + "showQuery": "Mostrar consulta", + "hideQuery": "Ocultar consulta", + "showExecutedQuery": "Mostrar consulta ejecutada", + "hideExecutedQuery": "Ocultar consulta ejecutada", "errorBoundary": { "title": "El editor falló inesperadamente", "description": "Algo en el editor no pudo renderizarse. El error se muestra abajo — vuelve a intentarlo o regresa a tus conexiones.", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index b0461734..b3beaf98 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -815,6 +815,10 @@ "queryFailed": "Échec de la requête.", "showErrorDetails": "Afficher les détails", "hideErrorDetails": "Masquer les détails", + "showQuery": "Afficher la requête", + "hideQuery": "Masquer la requête", + "showExecutedQuery": "Afficher la requête exécutée", + "hideExecutedQuery": "Masquer la requête exécutée", "errorBoundary": { "title": "L'éditeur a planté de manière inattendue", "description": "Un élément de l'éditeur n'a pas pu s'afficher. L'erreur est indiquée ci-dessous — réessayez ou revenez à vos connexions.", diff --git a/src/i18n/locales/it.json b/src/i18n/locales/it.json index 11927a3e..32dade17 100644 --- a/src/i18n/locales/it.json +++ b/src/i18n/locales/it.json @@ -798,6 +798,10 @@ "queryFailed": "Esecuzione query fallita.", "showErrorDetails": "Mostra dettagli", "hideErrorDetails": "Nascondi dettagli", + "showQuery": "Mostra query", + "hideQuery": "Nascondi query", + "showExecutedQuery": "Mostra query eseguita", + "hideExecutedQuery": "Nascondi query eseguita", "errorBoundary": { "title": "L'editor si è bloccato in modo imprevisto", "description": "Qualcosa nell'editor non è riuscito a renderizzarsi. L'errore è mostrato qui sotto — riprova oppure torna alle connessioni.", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index 843edc96..6411e077 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -828,6 +828,10 @@ "queryFailed": "クエリが失敗しました。", "showErrorDetails": "詳細を表示", "hideErrorDetails": "詳細を非表示", + "showQuery": "クエリを表示", + "hideQuery": "クエリを非表示", + "showExecutedQuery": "実行されたクエリを表示", + "hideExecutedQuery": "実行されたクエリを非表示", "errorBoundary": { "title": "エディタが予期せずクラッシュしました", "description": "エディタの一部のレンダリングに失敗しました。エラーは下に表示されています。再試行するか、接続一覧に戻ってください。", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index 7968c15e..a8ef7621 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -810,6 +810,10 @@ "queryFailed": "Ошибка выполнения запроса.", "showErrorDetails": "Показать подробности", "hideErrorDetails": "Скрыть подробности", + "showQuery": "Показать запрос", + "hideQuery": "Скрыть запрос", + "showExecutedQuery": "Показать выполненный запрос", + "hideExecutedQuery": "Скрыть выполненный запрос", "errorBoundary": { "title": "Редактор завершился с ошибкой", "description": "Ошибка при отображении редактора. Подробности ниже — попробуйте ещё раз или вернитесь к подключениям.", diff --git a/src/i18n/locales/zh.json b/src/i18n/locales/zh.json index 9aeba8ce..7380df8a 100644 --- a/src/i18n/locales/zh.json +++ b/src/i18n/locales/zh.json @@ -783,6 +783,10 @@ "queryFailed": "查询失败。", "showErrorDetails": "显示详情", "hideErrorDetails": "隐藏详情", + "showQuery": "显示查询", + "hideQuery": "隐藏查询", + "showExecutedQuery": "显示已执行的查询", + "hideExecutedQuery": "隐藏已执行的查询", "errorBoundary": { "title": "编辑器意外崩溃", "description": "编辑器中的某些内容渲染失败。错误显示如下 — 请重试或返回到你的连接列表。", diff --git a/src/pages/Editor.tsx b/src/pages/Editor.tsx index d63ce974..1483690c 100644 --- a/src/pages/Editor.tsx +++ b/src/pages/Editor.tsx @@ -2964,7 +2964,11 @@ export const Editor = () => {

{t("editor.executingQuery")}

) : activeTab.error ? ( - + ) : activeTab.result || (activeTab.pendingInsertions && Object.keys(activeTab.pendingInsertions).length > 0) ? (