From 8df91d8ec273378084397349360ed1284696a520 Mon Sep 17 00:00:00 2001 From: Seth Wiesman Date: Tue, 30 Jun 2026 11:55:58 -0500 Subject: [PATCH 1/3] mz-deploy: report statement offsets in raw-file coordinates Statement byte offsets were computed against the variable-resolved SQL, but every consumer (the CLI diagnostic renderer and the LSP) reads the raw file. A `:var` whose value differs in length from the reference shifted all later offsets, so diagnostics pointed at the wrong span and could miss the token entirely. Map each offset back to the raw file with the substitution table at parse time, fixing both consumers at the source. Ticket: DEX-60 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/mz-deploy/src/project/syntax/parser.rs | 39 ++++++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/mz-deploy/src/project/syntax/parser.rs b/src/mz-deploy/src/project/syntax/parser.rs index ba09ea9b43e05..408a964c32c9f 100644 --- a/src/mz-deploy/src/project/syntax/parser.rs +++ b/src/mz-deploy/src/project/syntax/parser.rs @@ -64,7 +64,7 @@ where pub struct LocatedStatement { /// The parsed AST node. pub ast: Statement, - /// Byte offset of the statement's start within the (resolved) SQL text. + /// Byte offset of the statement's start within the raw source file. pub byte_offset: usize, } @@ -105,6 +105,7 @@ pub(crate) fn parse_statements_with_context( } } + let substitutions = resolved.substitutions; let sql = resolved.sql; let mut statements = vec![]; @@ -130,17 +131,19 @@ pub(crate) fn parse_statements_with_context( // Because both pointers reference the same allocation, subtracting the // base pointer from the slice pointer yields a valid byte offset. // - // Note: offsets are relative to the *variable-resolved* SQL text (the - // `sql` local above), not the raw file contents. When the LSP converts - // these to line/column positions it must build the Rope from the same - // resolved text, or re-resolve variables before lookup. + // The raw subtraction yields an offset into the variable-resolved text. A + // `:var` whose value differs in length from the reference shifts every + // later offset, so map back to the raw file the consumers (CLI renderer, + // LSP) actually read. #[allow(clippy::as_conversions)] let sql_base = sql.as_ptr() as usize; let mut parsed: Vec = parsed_results .into_iter() .map(|result| { #[allow(clippy::as_conversions)] - let byte_offset = result.sql.as_ptr() as usize - sql_base; + let resolved_offset = result.sql.as_ptr() as usize - sql_base; + let byte_offset = + super::variables::resolved_to_original(resolved_offset, &substitutions); LocatedStatement { ast: result.ast, byte_offset, @@ -183,13 +186,35 @@ pub(crate) fn statement_type_name(stmt: &Statement) -> &'static str { #[cfg(test)] mod test { - use crate::project::syntax::parser::parse_statements; + use crate::project::syntax::parser::{parse_statements, parse_statements_with_context}; + use std::collections::BTreeMap; + use std::path::PathBuf; #[mz_ore::test] fn validate() { let _ = parse_statements(vec!["CREATE CLUSTER c (INTROSPECTION INTERVAL = 0)"]).unwrap(); } + #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` + #[mz_ore::test] + fn statement_offsets_are_relative_to_raw_text() { + // The first statement substitutes `:col` with a longer value, shifting + // every later offset in the resolved text. The reported offset must + // still index the raw file, where consumers read source. + let raw = "CREATE VIEW v AS SELECT :col;\nCREATE VIEW w AS SELECT 2;"; + let mut variables = BTreeMap::new(); + variables.insert("col".to_string(), "some_long_column_name".to_string()); + + let stmts = + parse_statements_with_context(raw, PathBuf::from("v.sql"), &variables, true).unwrap(); + assert_eq!(stmts.len(), 2); + assert!( + raw[stmts[1].byte_offset..].starts_with("CREATE VIEW w"), + "offset {} should index the second statement in raw text", + stmts[1].byte_offset, + ); + } + #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` #[mz_ore::test] fn test_mv_in_cluster() { From 3e1707936a24836f47ed4ec63dcc93083c9abe49 Mon Sep 17 00:00:00 2001 From: Seth Wiesman Date: Wed, 1 Jul 2026 09:14:57 -0500 Subject: [PATCH 2/3] Apply suggestion from @sjwiesman --- src/mz-deploy/src/project/syntax/parser.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/mz-deploy/src/project/syntax/parser.rs b/src/mz-deploy/src/project/syntax/parser.rs index 408a964c32c9f..5dd2c59756974 100644 --- a/src/mz-deploy/src/project/syntax/parser.rs +++ b/src/mz-deploy/src/project/syntax/parser.rs @@ -198,9 +198,6 @@ mod test { #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` #[mz_ore::test] fn statement_offsets_are_relative_to_raw_text() { - // The first statement substitutes `:col` with a longer value, shifting - // every later offset in the resolved text. The reported offset must - // still index the raw file, where consumers read source. let raw = "CREATE VIEW v AS SELECT :col;\nCREATE VIEW w AS SELECT 2;"; let mut variables = BTreeMap::new(); variables.insert("col".to_string(), "some_long_column_name".to_string()); From 8e89c8689296ce0124b4a3fd2cdcea7bfb0a3b9b Mon Sep 17 00:00:00 2001 From: Seth Wiesman Date: Tue, 21 Jul 2026 15:00:12 -0500 Subject: [PATCH 3/3] mz-deploy: test raw-offset clamp for statement-splitting variable values Add end-to-end coverage for the clamp branch in `resolved_to_original`, which was only exercised by unit tests with hand-built substitutions. A variable value containing `;` injects a statement separator absent from the raw file, so the following statement begins inside the substitution's resolved span. The new test asserts its offset clamps back to the reference and stays within the raw file's bounds. A second test covers cumulative delta across multiple substitutions before a statement. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/mz-deploy/src/project/syntax/parser.rs | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/mz-deploy/src/project/syntax/parser.rs b/src/mz-deploy/src/project/syntax/parser.rs index 5dd2c59756974..fcbecd949606e 100644 --- a/src/mz-deploy/src/project/syntax/parser.rs +++ b/src/mz-deploy/src/project/syntax/parser.rs @@ -212,6 +212,52 @@ mod test { ); } + #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` + #[mz_ore::test] + fn statement_offset_inside_substitution_clamps_to_reference() { + // A variable value containing `;` injects a statement separator absent + // from the raw file, so the following statement begins *inside* the + // substitution's resolved span. Its offset must clamp back to the `:x` + // reference and stay within the raw file's bounds rather than running + // off the end. + let raw = "SELECT :x;"; + let mut variables = BTreeMap::new(); + variables.insert("x".to_string(), "1; SELECT 2".to_string()); + + let stmts = + parse_statements_with_context(raw, PathBuf::from("v.sql"), &variables, true).unwrap(); + assert_eq!(stmts.len(), 2); + let offset = stmts[1].byte_offset; + assert_eq!(offset, raw.find(":x").unwrap()); + assert!( + offset < raw.len(), + "clamped offset {} must stay within raw bounds ({})", + offset, + raw.len(), + ); + assert!(raw[offset..].starts_with(":x")); + } + + #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` + #[mz_ore::test] + fn statement_offset_accounts_for_multiple_substitutions() { + // Two substitutions of differing length in the first statement build up + // a cumulative delta the second statement's raw offset must undo. + let raw = "SELECT :a + :b;\nSELECT 3;"; + let mut variables = BTreeMap::new(); + variables.insert("a".to_string(), "100".to_string()); + variables.insert("b".to_string(), "2000".to_string()); + + let stmts = + parse_statements_with_context(raw, PathBuf::from("v.sql"), &variables, true).unwrap(); + assert_eq!(stmts.len(), 2); + assert!( + raw[stmts[1].byte_offset..].starts_with("SELECT 3"), + "offset {} should index the second statement in raw text", + stmts[1].byte_offset, + ); + } + #[cfg_attr(miri, ignore)] // unsupported operation: can't call foreign function `rust_psm_stack_pointer` on OS `linux` #[mz_ore::test] fn test_mv_in_cluster() {