Skip to content

Commit 1d26df9

Browse files
authored
fix(cli): do not split BEGIN...END script blocks on inner semicolons (#786)
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 1d26df9

2 files changed

Lines changed: 241 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: 240 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,114 @@
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+
// Disable further detection so that AS keywords
83+
// inside the body (e.g. column aliases) are not
84+
// mistaken for the task-level AS.
85+
self.state = TaskBlockState::Init;
86+
}
87+
}
88+
}
89+
TaskBlockState::InBlock => {
90+
if token.kind == TokenKind::END
91+
&& self.previous_token_kind == Some(TokenKind::SemiColon)
92+
{
93+
self.state = TaskBlockState::SeenBlockEnd;
94+
}
95+
}
96+
TaskBlockState::SeenBlockEnd => {
97+
// After seeing `; END`, only `;`, the delimiter, `\`
98+
// (start of `\G`), or `G` after `\` should keep us in
99+
// this state. Anything else means it wasn't the real
100+
// block end.
101+
let is_backslash_g = token.kind == TokenKind::Ident
102+
&& token.text() == "G"
103+
&& self.previous_token_kind == Some(TokenKind::Backslash);
104+
if token.kind != TokenKind::SemiColon
105+
&& token.kind != TokenKind::Backslash
106+
&& !is_backslash_g
107+
&& token.text() != delimiter_str
108+
{
109+
self.state = TaskBlockState::InBlock;
110+
}
111+
}
112+
}
113+
114+
self.previous_token_kind = Some(token.kind);
115+
}
116+
117+
/// Returns true when the parser should skip delimiters (we are
118+
/// inside a task script block and haven't seen the closing END).
119+
fn in_block(&self) -> bool {
120+
self.state == TaskBlockState::InBlock
121+
}
122+
}
16123

17124
/// SQL parser utility for splitting SQL text into individual statements
18125
pub struct SqlParser {
@@ -244,29 +351,37 @@ impl SqlParser {
244351
let mut remaining_query = to_parse.to_string();
245352
let mut err = String::new();
246353

354+
let delimiter_str = self.delimiter.to_string();
355+
247356
'Parser: loop {
248357
let mut is_valid = true;
249358
let tokenizer = Tokenizer::new(&remaining_query);
250359
let mut previous_token_backslash = false;
360+
let mut tracker = TaskBlockTracker::new();
251361

252362
for token in tokenizer {
253363
match token {
254364
Ok(token) => {
365+
tracker.feed(&token, &delimiter_str);
366+
255367
// SQL end with `;` or `\G` in repl
256-
let is_end_query = token.text() == self.delimiter.to_string();
368+
let is_end_query = token.text() == delimiter_str;
257369
let is_slash_g = self.is_repl
258370
&& (previous_token_backslash
259371
&& token.kind == TokenKind::Ident
260372
&& token.text() == "G")
261373
|| (token.text().ends_with("\\G"));
262374

263375
if is_end_query || is_slash_g {
376+
if tracker.in_block() {
377+
// Skip inner delimiters within the block.
378+
previous_token_backslash =
379+
matches!(token.kind, TokenKind::Backslash);
380+
continue;
381+
}
264382
// Extract the statement and continue with remaining text
265383
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-
{
384+
if is_valid && !sql.is_empty() && sql.trim() != delimiter_str {
270385
let sql = sql.trim_end_matches(self.delimiter);
271386
statements.push(sql.trim().to_string());
272387
}
@@ -310,3 +425,122 @@ pub fn parse_sql_for_web(sql_text: &str) -> Vec<String> {
310425
let parser = SqlParser::new(';', true, false);
311426
parser.parse(sql_text)
312427
}
428+
429+
#[cfg(test)]
430+
mod tests {
431+
use super::*;
432+
433+
#[test]
434+
fn test_task_script_block() {
435+
let parser = SqlParser::new(';', true, false);
436+
// Block is kept as a single statement, and trailing SQL splits correctly
437+
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;";
438+
let stmts = parser.parse(sql);
439+
assert_eq!(stmts.len(), 2);
440+
assert!(stmts[0].contains("BEGIN"));
441+
assert!(stmts[0].contains("select 1;"));
442+
assert!(stmts[0].contains("select 2;"));
443+
// The outer ; is the client delimiter and gets trimmed.
444+
// Server's task_sql_block expects BEGIN...END without trailing ;.
445+
assert!(stmts[0].ends_with("END"), "got: {}", stmts[0]);
446+
assert_eq!(stmts[1], "SELECT 3");
447+
}
448+
449+
#[test]
450+
fn test_task_script_block_repl_line_by_line() {
451+
let parser = SqlParser::new(';', true, true);
452+
let mut buf = String::new();
453+
let mut err = String::new();
454+
455+
// Simulate line-by-line REPL input
456+
assert!(parser
457+
.parse_line("CREATE TASK t1 AS", &mut buf, &mut err)
458+
.is_empty());
459+
assert!(parser.parse_line("BEGIN", &mut buf, &mut err).is_empty());
460+
assert!(parser
461+
.parse_line(" select 1;", &mut buf, &mut err)
462+
.is_empty());
463+
assert!(parser
464+
.parse_line(" select 2;", &mut buf, &mut err)
465+
.is_empty());
466+
let stmts = parser.parse_line("END;", &mut buf, &mut err);
467+
assert_eq!(stmts.len(), 1);
468+
assert!(stmts[0].contains("BEGIN"));
469+
assert!(stmts[0].contains("END"));
470+
assert!(stmts[0].contains("select 1;"));
471+
assert!(buf.is_empty());
472+
}
473+
474+
#[test]
475+
fn test_task_script_block_with_inner_transaction() {
476+
// Task script block containing BEGIN (transaction) inside
477+
let parser = SqlParser::new(';', true, false);
478+
let sql = "CREATE TASK t1 AS\nBEGIN\n begin;\n select 1;\n commit;\nEND;";
479+
let stmts = parser.parse(sql);
480+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
481+
assert!(stmts[0].contains("BEGIN"));
482+
assert!(stmts[0].contains("begin;"));
483+
assert!(stmts[0].contains("select 1;"));
484+
assert!(stmts[0].contains("commit;"));
485+
assert!(stmts[0].ends_with("END"));
486+
}
487+
488+
#[test]
489+
fn test_task_script_block_with_case_expression() {
490+
// CASE...END; inside a task block should not close the block
491+
let parser = SqlParser::new(';', true, false);
492+
let sql = "CREATE TASK t1 AS\nBEGIN\n INSERT INTO t SELECT CASE WHEN flag THEN 1 ELSE 0 END;\n select 2;\nEND;";
493+
let stmts = parser.parse(sql);
494+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
495+
assert!(stmts[0].contains("CASE"));
496+
assert!(stmts[0].contains("select 2;"));
497+
assert!(stmts[0].ends_with("END"));
498+
}
499+
500+
#[test]
501+
fn test_task_script_block_no_trailing_semicolon() {
502+
// User omits the trailing ; after END
503+
let parser = SqlParser::new(';', true, false);
504+
let sql = "CREATE TASK t1 AS\nBEGIN\n select 1;\n select 2;\nEND";
505+
let stmts = parser.parse(sql);
506+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
507+
assert!(stmts[0].contains("BEGIN"));
508+
assert!(stmts[0].contains("END"));
509+
}
510+
511+
#[test]
512+
fn test_task_single_statement_with_as_begin_alias() {
513+
// CREATE TASK with single statement body containing AS begin alias
514+
let parser = SqlParser::new(';', true, false);
515+
let sql = "CREATE TASK t1 AS SELECT 1 AS begin; SELECT 2;";
516+
let stmts = parser.parse(sql);
517+
assert_eq!(stmts.len(), 2, "got: {:?}", stmts);
518+
assert_eq!(stmts[0], "CREATE TASK t1 AS SELECT 1 AS begin");
519+
assert_eq!(stmts[1], "SELECT 2");
520+
}
521+
522+
#[test]
523+
fn test_task_script_block_custom_delimiter() {
524+
// With a custom delimiter (|), END; is SQL syntax and | is client terminator
525+
let parser = SqlParser::new('|', true, false);
526+
let sql = "CREATE TASK t1 AS\nBEGIN\n select 1;\n select 2;\nEND;\n|\nSELECT 3|";
527+
let stmts = parser.parse(sql);
528+
assert_eq!(stmts.len(), 2, "got: {:?}", stmts);
529+
assert!(stmts[0].contains("BEGIN"));
530+
assert!(stmts[0].contains("select 1;"));
531+
assert!(stmts[0].contains("END;"));
532+
assert_eq!(stmts[1], "SELECT 3");
533+
}
534+
535+
#[test]
536+
fn test_task_script_block_backslash_g_terminator() {
537+
// \G as statement terminator should close a task block
538+
let parser = SqlParser::new(';', true, true);
539+
let sql = "CREATE TASK t1 AS\nBEGIN\n select 1;\n select 2;\nEND\\G";
540+
let stmts = parser.parse(sql);
541+
assert_eq!(stmts.len(), 1, "got: {:?}", stmts);
542+
assert!(stmts[0].contains("BEGIN"));
543+
assert!(stmts[0].contains("select 1;"));
544+
assert!(stmts[0].contains("select 2;"));
545+
}
546+
}

0 commit comments

Comments
 (0)