From 7f405eacfbb7fee8905542db6d53db45d4a37e60 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Tue, 11 Aug 2026 12:20:21 +0000 Subject: [PATCH] fix(sqlserver): accept cursor declarations in SQL analysis Closes #5860 --- .../src/lib/sql/semantic/diagnostics.ts | 65 +++++++- crates/dbx-core/src/sql_analysis.rs | 118 +++++++++++++- crates/dbx-core/tests/sql_analysis.rs | 146 ++++++++++++++++++ .../app-tests/sqlSemanticDiagnostics.test.ts | 50 ++++++ 4 files changed, 375 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/lib/sql/semantic/diagnostics.ts b/apps/desktop/src/lib/sql/semantic/diagnostics.ts index f17ffed40d..40b1bf384c 100644 --- a/apps/desktop/src/lib/sql/semantic/diagnostics.ts +++ b/apps/desktop/src/lib/sql/semantic/diagnostics.ts @@ -25,7 +25,8 @@ export interface SqlSemanticDiagnosticVisibleRange { } export function sqlSemanticDiagnosticRangesForViewport(sql: string, visibleRanges: readonly SqlSemanticDiagnosticVisibleRange[], databaseType?: DatabaseType): SqlTextRange[] { - const statements = executableStatementRanges(sql, databaseType); + const executableRanges = executableStatementRanges(sql, databaseType); + const statements = databaseType === "sqlserver" ? mergeSqlServerDiagnosticBatchRanges(sql, executableRanges) : executableRanges; if (statements.length === 0 || visibleRanges.length === 0) return []; const selected: SqlTextRange[] = []; @@ -41,6 +42,68 @@ export function sqlSemanticDiagnosticRangesForViewport(sql: string, visibleRange return selected; } +function mergeSqlServerDiagnosticBatchRanges(sql: string, statements: readonly SqlTextRange[]): SqlTextRange[] { + if (statements.length < 2) return [...statements]; + + const ranges: SqlTextRange[] = []; + let batch: SqlTextRange[] = [statements[0]]; + for (const statement of statements.slice(1)) { + if (sqlServerGapContainsGoSeparator(sql.slice(batch[batch.length - 1].to, statement.from))) { + pushSqlServerDiagnosticBatchRanges(ranges, sql, batch); + batch = []; + } + batch.push(statement); + } + pushSqlServerDiagnosticBatchRanges(ranges, sql, batch); + return ranges; +} + +function pushSqlServerDiagnosticBatchRanges(ranges: SqlTextRange[], sql: string, batch: readonly SqlTextRange[]) { + if (batch.length > 1 && batch.some((statement) => sqlServerStatementNeedsBatchContext(statement.sql))) { + const from = batch[0].from; + const to = batch[batch.length - 1].to; + ranges.push({ from, to, sql: sql.slice(from, to) }); + } else { + ranges.push(...batch); + } +} + +function sqlServerStatementNeedsBatchContext(sql: string): boolean { + return /^\s*(?:WHILE|BEGIN|END|IF|ELSE|TRY|CATCH)\b/i.test(sql) || /^\s*DECLARE\s+(?:\[[^\]]+\]|"[^"]+"|[A-Z_@#][\w@$#]*)\s+CURSOR\b/i.test(sql); +} + +function sqlServerGapContainsGoSeparator(gap: string): boolean { + let inBlockComment = false; + let lineStart = 0; + while (lineStart <= gap.length) { + const newline = gap.indexOf("\n", lineStart); + const lineEnd = newline < 0 ? gap.length : newline; + const line = gap.slice(lineStart, lineEnd).replace(/\r$/, ""); + if (!inBlockComment && /^\s*go(?:\s+\d+)?\s*$/i.test(line)) return true; + + let offset = 0; + while (offset < line.length) { + if (inBlockComment) { + const close = line.indexOf("*/", offset); + if (close < 0) break; + inBlockComment = false; + offset = close + 2; + continue; + } + const open = line.indexOf("/*", offset); + if (open < 0) break; + const lineComment = line.indexOf("--", offset); + if (lineComment >= 0 && lineComment < open) break; + inBlockComment = true; + offset = open + 2; + } + + if (newline < 0) break; + lineStart = newline + 1; + } + return false; +} + export function buildSqlSemanticDiagnostics(analysis: SqlReferenceAnalysis, schema: SqlSemanticDiagnosticSchema): SqlSemanticDiagnostic[] { const diagnostics: SqlSemanticDiagnostic[] = []; const tables = analysis.tables.filter((table) => table.name.trim()); diff --git a/crates/dbx-core/src/sql_analysis.rs b/crates/dbx-core/src/sql_analysis.rs index a92baaa71a..64280829de 100644 --- a/crates/dbx-core/src/sql_analysis.rs +++ b/crates/dbx-core/src/sql_analysis.rs @@ -127,7 +127,8 @@ fn parse_sqlserver(sql: &str) -> Result, ParserError> { Err(error) => { let mut fallback_tokens = Tokenizer::new(&dialect, sql).tokenize_with_location()?; normalize_sqlserver_create_proc_tokens(&mut fallback_tokens); - let mut changed = normalize_sqlserver_alter_table_add_tokens(&mut fallback_tokens); + let mut changed = normalize_sqlserver_cursor_tokens(&mut fallback_tokens); + changed |= normalize_sqlserver_alter_table_add_tokens(&mut fallback_tokens); changed |= remove_sqlserver_query_hint_tokens(&mut fallback_tokens); if changed { if let Ok(statements) = @@ -141,6 +142,109 @@ fn parse_sqlserver(sql: &str) -> Result, ParserError> { } } +fn normalize_sqlserver_cursor_tokens(tokens: &mut Vec) -> bool { + let mut removals = Vec::new(); + let mut index = 0usize; + + while let Some(statement_start) = next_significant_token_index(tokens, index) { + if matches!(tokens[statement_start].token, Token::EOF) { + break; + } + if matches!(tokens[statement_start].token, Token::SemiColon) { + index = statement_start + 1; + continue; + } + + let statement_end = sqlserver_statement_end_index(tokens, statement_start); + let significant_indexes = tokens + .iter() + .enumerate() + .take(statement_end) + .skip(statement_start) + .filter_map(|(index, token)| (!matches!(token.token, Token::Whitespace(_))).then_some(index)) + .collect::>(); + + if let Some(removal) = sqlserver_cursor_option_removal(tokens, &significant_indexes) { + removals.push(removal); + } + + index = statement_end.saturating_add(1); + } + + let changed = !removals.is_empty(); + for removal in removals.into_iter().rev() { + tokens.drain(removal); + } + changed +} + +fn sqlserver_cursor_option_removal(tokens: &[TokenWithSpan], indexes: &[usize]) -> Option> { + if indexes.len() < 4 + || !unquoted_token_word_eq(&tokens[indexes[0]], "DECLARE") + || !is_sqlserver_named_cursor(&tokens[indexes[1]]) + || !unquoted_token_word_eq(&tokens[indexes[2]], "CURSOR") + { + return None; + } + + let mut position = 3usize; + if indexes.get(position).is_some_and(|index| { + unquoted_token_word_eq(&tokens[*index], "LOCAL") || unquoted_token_word_eq(&tokens[*index], "GLOBAL") + }) { + position += 1; + } + if indexes.get(position).is_some_and(|index| unquoted_token_word_eq(&tokens[*index], "FAST_FORWARD")) { + position += 1; + } + if position == 3 || !indexes.get(position).is_some_and(|index| unquoted_token_word_eq(&tokens[*index], "FOR")) { + return None; + } + + let query_position = position + 1; + let query_start = *indexes.get(query_position)?; + if (!unquoted_token_word_eq(&tokens[query_start], "SELECT") + && !unquoted_token_word_eq(&tokens[query_start], "WITH")) + || sqlserver_cursor_query_has_forbidden_clause(tokens, &indexes[query_position..]) + { + return None; + } + + Some(indexes[3]..indexes[position]) +} + +fn is_sqlserver_named_cursor(token: &TokenWithSpan) -> bool { + let Token::Word(word) = &token.token else { + return false; + }; + !word.value.starts_with('@') && (word.quote_style.is_some() || word.keyword == Keyword::NoKeyword) +} + +fn sqlserver_cursor_query_has_forbidden_clause(tokens: &[TokenWithSpan], indexes: &[usize]) -> bool { + let mut depth = 0usize; + for (position, index) in indexes.iter().copied().enumerate() { + match tokens[index].token { + Token::LParen => depth += 1, + Token::RParen => depth = depth.saturating_sub(1), + _ if depth == 0 + && (unquoted_token_word_eq(&tokens[index], "INTO") + || unquoted_token_word_eq(&tokens[index], "COMPUTE") + || unquoted_token_word_eq(&tokens[index], "FOR") + && indexes + .get(position + 1) + .is_some_and(|index| unquoted_token_word_eq(&tokens[*index], "BROWSE"))) => + { + return true; + } + _ => {} + } + } + false +} + +fn unquoted_token_word_eq(token: &TokenWithSpan, expected: &str) -> bool { + matches!(&token.token, Token::Word(word) if word.quote_style.is_none() && word.value.eq_ignore_ascii_case(expected)) +} + fn normalize_sqlserver_alter_table_add_tokens(tokens: &mut Vec) -> bool { let mut insertions = Vec::new(); let mut index = 0usize; @@ -477,8 +581,16 @@ fn normalize_dialect(dialect: Option<&str>) -> String { impl Analyzer { fn visit_statement(&mut self, statement: &Statement) { - if let Statement::Query(query) = statement { - self.visit_query_in_new_scope(query, None); + match statement { + Statement::Query(query) => self.visit_query_in_new_scope(query, None), + Statement::Declare { stmts } if self.is_sqlserver => { + for declaration in stmts { + if let Some(query) = &declaration.for_query { + self.visit_query_in_new_scope(query, None); + } + } + } + _ => {} } } diff --git a/crates/dbx-core/tests/sql_analysis.rs b/crates/dbx-core/tests/sql_analysis.rs index e5adda1361..b31ed255be 100644 --- a/crates/dbx-core/tests/sql_analysis.rs +++ b/crates/dbx-core/tests/sql_analysis.rs @@ -410,6 +410,152 @@ fn sqlserver_proc_identifiers_remain_identifiers_outside_create() { assert_eq!(analysis.columns[0].name, "proc"); } +#[test] +fn sqlserver_cursor_declaration_analyzes_its_query_without_a_cursor_table() { + let sql = "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR + SELECT s.name FROM sys.schemas s WHERE s.schema_id > 0; + OPEN schema_cursor; + FETCH NEXT FROM schema_cursor INTO @schema; + CLOSE schema_cursor; + DEALLOCATE schema_cursor;"; + let analysis = analyze_sql_references(sql, Some("sqlserver")) + .unwrap_or_else(|error| panic!("valid SQL Server cursor declaration should analyze: {error}")); + + let tables: Vec<_> = analysis.tables.iter().map(|table| (table.schema.as_deref(), table.name.as_str())).collect(); + assert_eq!(tables, vec![(Some("sys"), "schemas")]); + assert!(analysis.tables.iter().all(|table| table.name != "schema_cursor")); +} + +#[test] +fn sqlserver_reported_cursor_query_does_not_raise_parser_or_table_errors() { + let analysis = analyze_sql_references( + "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR + SELECT N'dbo' UNION ALL SELECT N'dev';", + Some("sqlserver"), + ) + .expect("the reported SQL Server cursor declaration should analyze"); + + assert!(analysis.tables.is_empty()); + assert!(analysis.columns.is_empty()); +} + +#[test] +fn sqlserver_cursor_fallback_is_limited_to_the_reported_option_subset() { + for sql in [ + "DECLARE plain_cursor CURSOR FOR SELECT name FROM dbo.reports;", + "DECLARE local_cursor CURSOR LOCAL FOR SELECT name FROM dbo.reports;", + "DECLARE fast_cursor CURSOR FAST_FORWARD FOR SELECT name FROM dbo.reports;", + "DECLARE global_cursor CURSOR GLOBAL FAST_FORWARD FOR SELECT name FROM dbo.reports;", + "DECLARE json_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT (SELECT TOP 1 a.note FROM dbo.audit a FOR JSON PATH) AS payload FROM dbo.reports;", + ] { + let analysis = analyze_sql_references(sql, Some("sqlserver")) + .unwrap_or_else(|error| panic!("the reported cursor option subset should analyze: {error}")); + assert!(analysis.tables.iter().any(|table| table.name == "reports")); + } + + for sql in [ + "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD;", + "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR UPDATE dbo.reports SET name = 'invalid';", + "DECLARE report_cursor CURSOR SCROLL FAST_FORWARD FOR SELECT name FROM dbo.reports;", + "DECLARE report_cursor CURSOR LOCAL KEYSET FOR SELECT name FROM dbo.reports;", + "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD READ_ONLY FOR SELECT name FROM dbo.reports;", + "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name INTO #report_copy FROM dbo.reports;", + "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name FROM dbo.reports FOR BROWSE;", + ] { + if analyze_sql_references(sql, Some("sqlserver")).is_ok() { + panic!("unsupported or invalid cursor syntax must not be suppressed: {sql}"); + } + } +} + +#[test] +fn sqlserver_cursor_uses_native_lifecycle_statements_and_preserves_query_spans() { + for sql in [ + "OPEN schema_cursor", + "FETCH NEXT FROM schema_cursor INTO @schema", + "CLOSE schema_cursor", + "DEALLOCATE schema_cursor", + ] { + analyze_sql_references(sql, Some("sqlserver")) + .unwrap_or_else(|error| panic!("sqlparser should parse ordinary cursor lifecycle SQL {sql:?}: {error}")); + } + + let analysis = analyze_sql_references( + "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR\n SELECT r.name FROM dbo.reports r OPTION (RECOMPILE);", + Some("sqlserver"), + ) + .expect("cursor option and query-hint fallbacks should compose"); + assert_eq!(analysis.tables[0].name, "reports"); + assert_eq!(analysis.tables[0].span.start_line, 2); + assert_eq!(analysis.tables[0].span.start_column, 28); + assert_eq!(analysis.columns[0].name, "name"); + assert_eq!(analysis.columns[0].span.start_line, 2); + assert_eq!(analysis.columns[0].span.start_column, 14); + + analyze_sql_references( + "DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name FROM dbo.reports OPTION (RECOMPILE);\ + ALTER TABLE dbo.demo ADD first_flag BIT NULL, second_flag BIT NULL;", + Some("sqlserver"), + ) + .expect("cursor, query-hint, and ALTER TABLE fallbacks should compose"); +} + +#[test] +fn sqlserver_cursor_fallback_is_dialect_scoped_and_preserves_other_statements() { + let cursor_sql = "DECLARE report_cursor CURSOR LOCAL FAST_FORWARD FOR SELECT name FROM dbo.reports;"; + analyze_sql_references(cursor_sql, Some("postgres")) + .expect_err("other dialects must not inherit SQL Server cursor handling"); + + let analysis = + analyze_sql_references("DECLARE @schema SYSNAME; SELECT u.name FROM dbo.users u;", Some("sqlserver")) + .expect("ordinary SQL Server variable declarations and table references should remain parseable"); + assert_eq!(analysis.tables.len(), 1); + assert_eq!(analysis.tables[0].schema.as_deref(), Some("dbo")); + assert_eq!(analysis.tables[0].name, "users"); +} + +#[test] +fn sqlserver_reported_cursor_batch_analyzes_with_control_flow_and_go() { + use dbx_core::sql::split_sql_batches; + + let sql = "IF SCHEMA_ID(N'dev') IS NULL + EXEC(N'CREATE SCHEMA dev AUTHORIZATION dbo'); +GO + +IF COL_LENGTH(N'dbo.sys_user', N'highStandarUser') IS NULL + ALTER TABLE dbo.sys_user ADD highStandarUser INT NOT NULL DEFAULT 0; +GO + +DECLARE @schema SYSNAME; +DECLARE @sql NVARCHAR(MAX); +DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR + SELECT N'dbo' UNION ALL SELECT N'dev'; + +OPEN schema_cursor; +FETCH NEXT FROM schema_cursor INTO @schema; + +WHILE @@FETCH_STATUS = 0 +BEGIN + IF OBJECT_ID(QUOTENAME(@schema) + N'.dashboard', N'U') IS NULL + BEGIN + SET @sql = N'CREATE TABLE dbo.dashboard (id INT);'; + EXEC sys.sp_executesql @sql; + END; + FETCH NEXT FROM schema_cursor INTO @schema; +END; + +CLOSE schema_cursor; +DEALLOCATE schema_cursor; +GO"; + + let batches = split_sql_batches(sql); + assert_eq!(batches.len(), 3); + for batch in batches { + analyze_sql_references(&batch, Some("sqlserver")) + .unwrap_or_else(|error| panic!("the reported SQL Server batch should analyze after removing GO: {error}")); + } +} + #[test] fn sqlserver_alter_table_single_add_supports_multiple_columns() { for sql in [ diff --git a/packages/app-tests/sqlSemanticDiagnostics.test.ts b/packages/app-tests/sqlSemanticDiagnostics.test.ts index fd5b3a64dd..41c0765ded 100644 --- a/packages/app-tests/sqlSemanticDiagnostics.test.ts +++ b/packages/app-tests/sqlSemanticDiagnostics.test.ts @@ -444,6 +444,56 @@ test("selects complete SQL statements intersecting the visible viewport for diag assert.equal(ranges[0]?.to, sql.indexOf(";\nSELECT * FROM third")); }); +test("analyzes SQL Server control flow as complete GO batches", () => { + const sql = `IF SCHEMA_ID(N'dev') IS NULL + EXEC(N'CREATE SCHEMA dev AUTHORIZATION dbo'); +GO + +DECLARE @schema SYSNAME; +DECLARE @sql NVARCHAR(MAX); +DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD FOR + SELECT N'dbo' UNION ALL SELECT N'dev'; +OPEN schema_cursor; +FETCH NEXT FROM schema_cursor INTO @schema; +WHILE @@FETCH_STATUS = 0 +BEGIN + IF OBJECT_ID(QUOTENAME(@schema) + N'.dashboard', N'U') IS NULL + BEGIN + SET @sql = N'CREATE TABLE dbo.dashboard (id INT);'; + EXEC sys.sp_executesql @sql; + END; + FETCH NEXT FROM schema_cursor INTO @schema; +END; +CLOSE schema_cursor; +DEALLOCATE schema_cursor; +GO`; + + const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: 0, to: sql.length }], "sqlserver"); + + assert.equal(ranges.length, 2); + assert.equal(ranges[0]?.sql, "IF SCHEMA_ID(N'dev') IS NULL\n EXEC(N'CREATE SCHEMA dev AUTHORIZATION dbo')"); + assert.equal(ranges[1]?.from, sql.indexOf("DECLARE @schema")); + assert.equal(ranges[1]?.to, sql.lastIndexOf(";\nGO")); + assert.match(ranges[1]?.sql ?? "", /DECLARE schema_cursor CURSOR LOCAL FAST_FORWARD/); + assert.match(ranges[1]?.sql ?? "", /WHILE @@FETCH_STATUS = 0[\s\S]*FETCH NEXT FROM schema_cursor INTO @schema;\nEND/); + assert.doesNotMatch(ranges[1]?.sql ?? "", /^\s*GO\s*$/m); + + const commentedGo = "DECLARE c CURSOR LOCAL FAST_FORWARD FOR SELECT 1;\n/*\nGO\n*/\nOPEN c;"; + assert.equal(sqlSemanticDiagnosticRangesForViewport(commentedGo, [{ from: 0, to: commentedGo.length }], "sqlserver").length, 1); +}); + +test("keeps ordinary SQL Server statements viewport-local", () => { + const sql = "SELECT id FROM first;\nSELECT id, missing_field FROM second;\nSELECT id FROM third;"; + const visibleFrom = sql.indexOf("missing_field"); + + const ranges = sqlSemanticDiagnosticRangesForViewport(sql, [{ from: visibleFrom, to: visibleFrom + 1 }], "sqlserver"); + + assert.deepEqual( + ranges.map((range) => range.sql), + ["SELECT id, missing_field FROM second"], + ); +}); + test("skips Oracle PL/SQL blocks when selecting semantic diagnostic ranges", () => { const sql = `DECLARE v_order_count NUMBER;