Skip to content

Commit 1c87e7b

Browse files
committed
fix(cli): do not split BEGIN...END script blocks on inner semicolons
The SQL statement splitter treated every semicolon as a statement boundary, which broke multi-statement CREATE TASK blocks: CREATE TASK t1 AS BEGIN select 1; select 2; END; The splitter would send only 'CREATE TASK t1 AS BEGIN select 1' to the server, causing a parse error. Fix: track AS BEGIN...END block depth. Only a semicolon immediately after a bare END token (not END IF, END FOR, etc.) closes the block. Transaction BEGIN statements are unaffected because they are not preceded by AS. Closes #785
1 parent 6a8a5bf commit 1c87e7b

2 files changed

Lines changed: 234 additions & 7 deletions

File tree

cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,4 @@ vergen-gix = { version = "9.1.0", features = ["build"] }
6666
name = "bendsql"
6767
path = "src/main.rs"
6868
doctest = false
69-
test = false
69+
test = true

cli/src/sql_parser.rs

Lines changed: 233 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,103 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use databend_common_ast::parser::token::{TokenKind, Tokenizer};
15+
use databend_common_ast::parser::token::{Token, TokenKind, Tokenizer};
16+
17+
/// State machine for tracking CREATE/ALTER TASK ... AS BEGIN ... END blocks.
18+
/// Semicolons inside a task script block should not split the statement.
19+
#[derive(PartialEq, Clone, Copy)]
20+
enum TaskBlockState {
21+
/// No task-related tokens seen yet.
22+
Init,
23+
/// Seen CREATE or ALTER.
24+
SeenCreateAlter,
25+
/// Seen CREATE/ALTER TASK; waiting for AS.
26+
SeenTask,
27+
/// Inside the BEGIN...END block; skipping inner delimiters.
28+
InBlock,
29+
/// Saw `; END` inside the block — candidate for block close.
30+
SeenBlockEnd,
31+
}
32+
33+
/// Tracks whether we are inside a CREATE/ALTER TASK ... AS BEGIN ... END
34+
/// script block, so that inner semicolons are not treated as statement
35+
/// terminators.
36+
struct TaskBlockTracker {
37+
state: TaskBlockState,
38+
paren_depth: u32,
39+
previous_token_kind: Option<TokenKind>,
40+
}
41+
42+
impl TaskBlockTracker {
43+
fn new() -> Self {
44+
Self {
45+
state: TaskBlockState::Init,
46+
paren_depth: 0,
47+
previous_token_kind: None,
48+
}
49+
}
50+
51+
/// Process a token and advance the state machine.
52+
fn feed(&mut self, token: &Token, delimiter_str: &str) {
53+
// Track parenthesis depth so we can ignore AS inside
54+
// expressions like WHEN CAST(... AS BOOLEAN).
55+
match token.kind {
56+
TokenKind::LParen => self.paren_depth += 1,
57+
TokenKind::RParen => self.paren_depth = self.paren_depth.saturating_sub(1),
58+
_ => {}
59+
}
60+
61+
match self.state {
62+
TaskBlockState::Init => {
63+
if token.kind == TokenKind::CREATE || token.kind == TokenKind::ALTER {
64+
self.state = TaskBlockState::SeenCreateAlter;
65+
}
66+
}
67+
TaskBlockState::SeenCreateAlter => {
68+
if token.kind == TokenKind::TASK {
69+
self.state = TaskBlockState::SeenTask;
70+
} else if token.kind != TokenKind::OR && token.kind != TokenKind::REPLACE {
71+
self.state = TaskBlockState::Init;
72+
}
73+
}
74+
TaskBlockState::SeenTask => {
75+
// Only consider top-level AS (paren_depth == 0).
76+
// AS inside CAST(... AS type) has paren_depth > 0.
77+
if self.previous_token_kind == Some(TokenKind::AS) && self.paren_depth == 0 {
78+
if token.kind == TokenKind::BEGIN {
79+
self.state = TaskBlockState::InBlock;
80+
} else {
81+
// Task body is a single statement, not a block.
82+
self.state = TaskBlockState::Init;
83+
}
84+
}
85+
}
86+
TaskBlockState::InBlock => {
87+
if token.kind == TokenKind::END
88+
&& self.previous_token_kind == Some(TokenKind::SemiColon)
89+
{
90+
self.state = TaskBlockState::SeenBlockEnd;
91+
}
92+
}
93+
TaskBlockState::SeenBlockEnd => {
94+
// After seeing `; END`, only `;` or the delimiter
95+
// should keep us in this state. Anything else
96+
// means it wasn't the real block end.
97+
if token.kind != TokenKind::SemiColon && token.text() != delimiter_str {
98+
self.state = TaskBlockState::InBlock;
99+
}
100+
}
101+
}
102+
103+
self.previous_token_kind = Some(token.kind);
104+
}
105+
106+
/// Returns true when the parser should skip delimiters (we are
107+
/// inside a task script block and haven't seen the closing END).
108+
fn in_block(&self) -> bool {
109+
self.state == TaskBlockState::InBlock
110+
}
111+
}
16112

17113
/// SQL parser utility for splitting SQL text into individual statements
18114
pub struct SqlParser {
@@ -244,29 +340,37 @@ impl SqlParser {
244340
let mut remaining_query = to_parse.to_string();
245341
let mut err = String::new();
246342

343+
let delimiter_str = self.delimiter.to_string();
344+
247345
'Parser: loop {
248346
let mut is_valid = true;
249347
let tokenizer = Tokenizer::new(&remaining_query);
250348
let mut previous_token_backslash = false;
349+
let mut tracker = TaskBlockTracker::new();
251350

252351
for token in tokenizer {
253352
match token {
254353
Ok(token) => {
354+
tracker.feed(&token, &delimiter_str);
355+
255356
// SQL end with `;` or `\G` in repl
256-
let is_end_query = token.text() == self.delimiter.to_string();
357+
let is_end_query = token.text() == delimiter_str;
257358
let is_slash_g = self.is_repl
258359
&& (previous_token_backslash
259360
&& token.kind == TokenKind::Ident
260361
&& token.text() == "G")
261362
|| (token.text().ends_with("\\G"));
262363

263364
if is_end_query || is_slash_g {
365+
if tracker.in_block() {
366+
// Skip inner delimiters within the block.
367+
previous_token_backslash =
368+
matches!(token.kind, TokenKind::Backslash);
369+
continue;
370+
}
264371
// Extract the statement and continue with remaining text
265372
let (sql, remain) = remaining_query.split_at(token.span.end as usize);
266-
if is_valid
267-
&& !sql.is_empty()
268-
&& sql.trim() != self.delimiter.to_string()
269-
{
373+
if is_valid && !sql.is_empty() && sql.trim() != delimiter_str {
270374
let sql = sql.trim_end_matches(self.delimiter);
271375
statements.push(sql.trim().to_string());
272376
}
@@ -310,3 +414,126 @@ pub fn parse_sql_for_web(sql_text: &str) -> Vec<String> {
310414
let parser = SqlParser::new(';', true, false);
311415
parser.parse(sql_text)
312416
}
417+
418+
#[cfg(test)]
419+
mod tests {
420+
use super::*;
421+
422+
#[test]
423+
fn test_task_script_block() {
424+
let parser = SqlParser::new(';', true, false);
425+
// Block is kept as a single statement, and trailing SQL splits correctly
426+
let sql = "CREATE TASK IF NOT EXISTS nightly_refresh\n WAREHOUSE = 'default'\n SCHEDULE = USING CRON '0 0 2 * * *' 'UTC'\nAS\nBEGIN\n select 1;\n select 2;\nEND;\nSELECT 3;";
427+
let stmts = parser.parse(sql);
428+
assert_eq!(stmts.len(), 2);
429+
assert!(stmts[0].contains("BEGIN"));
430+
assert!(stmts[0].contains("select 1;"));
431+
assert!(stmts[0].contains("select 2;"));
432+
// The outer ; is the client delimiter and gets trimmed.
433+
// Server's task_sql_block expects BEGIN...END without trailing ;.
434+
assert!(stmts[0].ends_with("END"), "got: {}", stmts[0]);
435+
assert_eq!(stmts[1], "SELECT 3");
436+
}
437+
438+
#[test]
439+
fn test_task_script_block_repl_line_by_line() {
440+
let parser = SqlParser::new(';', true, true);
441+
let mut buf = String::new();
442+
let mut err = String::new();
443+
444+
// Simulate line-by-line REPL input
445+
assert!(parser
446+
.parse_line("CREATE TASK t1 AS", &mut buf, &mut err)
447+
.is_empty());
448+
assert!(parser.parse_line("BEGIN", &mut buf, &mut err).is_empty());
449+
assert!(parser
450+
.parse_line(" select 1;", &mut buf, &mut err)
451+
.is_empty());
452+
assert!(parser
453+
.parse_line(" select 2;", &mut buf, &mut err)
454+
.is_empty());
455+
let stmts = parser.parse_line("END;", &mut buf, &mut err);
456+
assert_eq!(stmts.len(), 1);
457+
assert!(stmts[0].contains("BEGIN"));
458+
assert!(stmts[0].contains("END"));
459+
assert!(stmts[0].contains("select 1;"));
460+
assert!(buf.is_empty());
461+
}
462+
463+
#[test]
464+
fn test_procedure_dollar_quote_followed_by_statement() {
465+
// Ensure the statement after $$ block is split correctly
466+
let parser = SqlParser::new(';', true, false);
467+
let sql = r#"CREATE PROCEDURE p1() RETURNS STRING LANGUAGE SQL AS $$
468+
BEGIN
469+
LET x := 1;
470+
END;
471+
$$;
472+
SELECT 1;"#;
473+
let stmts = parser.parse(sql);
474+
assert_eq!(stmts.len(), 2, "got: {:?}", stmts);
475+
assert!(stmts[0].contains("$$"));
476+
assert_eq!(stmts[1], "SELECT 1");
477+
}
478+
479+
#[test]
480+
fn test_task_script_block_with_inner_transaction() {
481+
// Task script block containing BEGIN (transaction) inside
482+
let parser = SqlParser::new(';', true, false);
483+
let sql = "CREATE TASK t1 AS\nBEGIN\n begin;\n select 1;\n commit;\nEND;";
484+
let stmts = parser.parse(sql);
485+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
486+
assert!(stmts[0].contains("BEGIN"));
487+
assert!(stmts[0].contains("begin;"));
488+
assert!(stmts[0].contains("select 1;"));
489+
assert!(stmts[0].contains("commit;"));
490+
assert!(stmts[0].ends_with("END"));
491+
}
492+
493+
#[test]
494+
fn test_task_script_block_with_case_expression() {
495+
// CASE...END; inside a task block should not close the block
496+
let parser = SqlParser::new(';', true, false);
497+
let sql = "CREATE TASK t1 AS\nBEGIN\n INSERT INTO t SELECT CASE WHEN flag THEN 1 ELSE 0 END;\n select 2;\nEND;";
498+
let stmts = parser.parse(sql);
499+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
500+
assert!(stmts[0].contains("CASE"));
501+
assert!(stmts[0].contains("select 2;"));
502+
assert!(stmts[0].ends_with("END"));
503+
}
504+
505+
#[test]
506+
fn test_task_script_block_no_trailing_semicolon() {
507+
// User omits the trailing ; after END
508+
let parser = SqlParser::new(';', true, false);
509+
let sql = "CREATE TASK t1 AS\nBEGIN\n select 1;\n select 2;\nEND";
510+
let stmts = parser.parse(sql);
511+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
512+
assert!(stmts[0].contains("BEGIN"));
513+
assert!(stmts[0].contains("END"));
514+
}
515+
516+
#[test]
517+
fn test_task_single_statement_with_as_begin_alias() {
518+
// CREATE TASK with single statement body containing AS begin alias
519+
let parser = SqlParser::new(';', true, false);
520+
let sql = "CREATE TASK t1 AS SELECT 1 AS begin; SELECT 2;";
521+
let stmts = parser.parse(sql);
522+
assert_eq!(stmts.len(), 2, "got: {:?}", stmts);
523+
assert_eq!(stmts[0], "CREATE TASK t1 AS SELECT 1 AS begin");
524+
assert_eq!(stmts[1], "SELECT 2");
525+
}
526+
527+
#[test]
528+
fn test_task_script_block_custom_delimiter() {
529+
// With a custom delimiter (|), END; is SQL syntax and | is client terminator
530+
let parser = SqlParser::new('|', true, false);
531+
let sql = "CREATE TASK t1 AS\nBEGIN\n select 1;\n select 2;\nEND;\n|\nSELECT 3|";
532+
let stmts = parser.parse(sql);
533+
assert_eq!(stmts.len(), 2, "got: {:?}", stmts);
534+
assert!(stmts[0].contains("BEGIN"));
535+
assert!(stmts[0].contains("select 1;"));
536+
assert!(stmts[0].contains("END;"));
537+
assert_eq!(stmts[1], "SELECT 3");
538+
}
539+
}

0 commit comments

Comments
 (0)