Skip to content

Commit 4d786ca

Browse files
Trigger splitter: track CASE..END and skip preamble comments
Two follow-ups to the recent CREATE TRIGGER body splitter. CASE..END expressions inside trigger bodies use the same ``END`` keyword as the BEGIN..END block; the depth tracker decremented on every ``END`` and prematurely flipped ``in_trigger_body`` to False on the inner CASE close. The trailing ``;`` then split the DDL mid-statement and the bare outer ``END`` matched the COMMIT/END branch in the transaction tracker — exactly the desync the trigger splitter was meant to prevent. Add a ``case_depth`` counter scoped to the trigger body. ``CASE`` increments; ``END`` decrements ``case_depth`` first if non-zero, otherwise decrements ``trigger_depth``. Nested CASE expressions unwind correctly. Migration tools (Alembic, Flyway, Liquibase change-log markers) routinely interleave ``--`` line and ``/* */`` block comments between top-level keywords. The trigger preamble scanner only consumed whitespace, so ``CREATE/* migration v3 */TRIGGER ...`` silently bypassed the trigger-body-mode entry. Factor out a ``_skip_ws_and_comments`` helper and use it in the preamble (where the post-TRIGGER body scan already had full comment handling).
1 parent f14e7aa commit 4d786ca

2 files changed

Lines changed: 138 additions & 11 deletions

File tree

src/dqliteclient/connection.py

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,15 @@ def _split_top_level_statements(sql: str) -> list[str]:
144144
# ``in_trigger_body``: inside a ``CREATE TRIGGER ... BEGIN`` body.
145145
# ``trigger_depth``: inner BEGIN..END nesting count (for compound
146146
# SQLite trigger bodies, e.g. ``BEGIN ... BEGIN ... END; END;``).
147+
# ``case_depth``: open ``CASE`` expressions inside the body. ``END``
148+
# closes the innermost CASE first; only when no CASE is open does
149+
# ``END`` decrement ``trigger_depth``. Without this counter, an
150+
# inner ``CASE WHEN ... END`` would prematurely flip
151+
# ``in_trigger_body`` to False and the trailing ``;`` would split
152+
# the DDL mid-statement.
147153
in_trigger_body = False
148154
trigger_depth = 0
155+
case_depth = 0
149156
# Position in the current statement piece where we last checked
150157
# for a CREATE TRIGGER preamble — only check ahead of, not behind,
151158
# to keep this O(n) per piece.
@@ -223,15 +230,24 @@ def _split_top_level_statements(sql: str) -> list[str]:
223230
else:
224231
# Inside trigger body — track nested BEGIN..END so a
225232
# compound trigger (BEGIN ... BEGIN ... END; END;)
226-
# closes correctly.
233+
# closes correctly. Also track CASE expressions so
234+
# the inner ``END`` of ``CASE WHEN ... END`` does not
235+
# decrement ``trigger_depth``.
227236
if kw == "BEGIN":
228237
trigger_depth += 1
229238
i = kw_end
230239
continue
240+
if kw == "CASE":
241+
case_depth += 1
242+
i = kw_end
243+
continue
231244
if kw == "END":
232-
trigger_depth -= 1
233-
if trigger_depth == 0:
234-
in_trigger_body = False
245+
if case_depth > 0:
246+
case_depth -= 1
247+
else:
248+
trigger_depth -= 1
249+
if trigger_depth == 0:
250+
in_trigger_body = False
235251
i = kw_end
236252
continue
237253
i = kw_end
@@ -256,6 +272,32 @@ def _is_word_char(c: str) -> bool:
256272
return c.isalnum() or c == "_"
257273

258274

275+
def _skip_ws_and_comments(sql: str, i: int, n: int) -> int:
276+
"""Advance past whitespace and SQL comments (``--`` line and
277+
``/* */`` block).
278+
279+
Used by the trigger-preamble scan so migration-tool output that
280+
interleaves comments between top-level keywords (e.g.
281+
``CREATE/* migration v3 */TRIGGER``) parses correctly. Stops at
282+
end-of-input or at any non-whitespace, non-comment character.
283+
"""
284+
while i < n:
285+
c = sql[i]
286+
if c.isspace():
287+
i += 1
288+
continue
289+
if c == "-" and i + 1 < n and sql[i + 1] == "-":
290+
nl = sql.find("\n", i + 2)
291+
i = n if nl == -1 else nl + 1
292+
continue
293+
if c == "/" and i + 1 < n and sql[i + 1] == "*":
294+
end = sql.find("*/", i + 2)
295+
i = n if end == -1 else end + 2
296+
continue
297+
break
298+
return i
299+
300+
259301
def _scan_for_trigger_begin(sql: str, after_create: int, n: int) -> int:
260302
"""Look ahead from just after ``CREATE`` for the trigger preamble
261303
``[TEMP|TEMPORARY] TRIGGER ... BEGIN`` and return the index just
@@ -269,20 +311,20 @@ def _scan_for_trigger_begin(sql: str, after_create: int, n: int) -> int:
269311
constructs by the time the alpha-token branch fires).
270312
"""
271313
i = after_create
272-
# Optional whitespace, then optional TEMP/TEMPORARY, then required
273-
# TRIGGER keyword.
274-
while i < n and sql[i].isspace():
275-
i += 1
314+
# Optional whitespace + comments, then optional TEMP/TEMPORARY,
315+
# then required TRIGGER keyword. Comment-handling matches the
316+
# post-TRIGGER body scan so migration-tool output that places
317+
# ``/* ... */`` markers between top-level keywords parses
318+
# correctly.
319+
i = _skip_ws_and_comments(sql, i, n)
276320
if i >= n:
277321
return 0
278322
j = i
279323
while j < n and _is_word_char(sql[j]):
280324
j += 1
281325
word = sql[i:j].upper()
282326
if word in ("TEMP", "TEMPORARY"):
283-
i = j
284-
while i < n and sql[i].isspace():
285-
i += 1
327+
i = _skip_ws_and_comments(sql, j, n)
286328
j = i
287329
while j < n and _is_word_char(sql[j]):
288330
j += 1
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
"""Pin: trigger splitter handles CASE...END expressions and comments
2+
between CREATE and TRIGGER.
3+
4+
Two cycle-19 follow-ups:
5+
6+
1. ``CASE WHEN ... END`` inside a trigger body must NOT decrement
7+
``trigger_depth`` (the END terminates the CASE expression, not
8+
the BEGIN..END block).
9+
2. Comments (``--`` line, ``/* */`` block) between ``CREATE`` and
10+
``TRIGGER`` (or between ``TEMP`` / ``TEMPORARY`` and ``TRIGGER``)
11+
must be skipped — migration-tool output routinely interleaves
12+
metadata comments at this position.
13+
"""
14+
15+
from __future__ import annotations
16+
17+
import pytest
18+
19+
from dqliteclient.connection import _split_top_level_statements
20+
21+
22+
class TestTriggerBodyCaseExpression:
23+
def test_simple_case_in_update_kept_together(self) -> None:
24+
sql = (
25+
"CREATE TRIGGER aud AFTER UPDATE ON t BEGIN\n"
26+
" UPDATE t SET x = CASE WHEN y > 0 THEN 1 ELSE 2 END;\n"
27+
"END;"
28+
)
29+
pieces = _split_top_level_statements(sql)
30+
assert len(pieces) == 1
31+
32+
def test_nested_case(self) -> None:
33+
sql = (
34+
"CREATE TRIGGER aud AFTER UPDATE ON t BEGIN\n"
35+
" UPDATE t SET x = CASE WHEN y > 0 THEN "
36+
"(CASE WHEN z THEN 'a' ELSE 'b' END) ELSE 'c' END;\n"
37+
"END;"
38+
)
39+
pieces = _split_top_level_statements(sql)
40+
assert len(pieces) == 1
41+
42+
def test_multiple_cases_in_body(self) -> None:
43+
sql = (
44+
"CREATE TRIGGER aud AFTER UPDATE ON t BEGIN\n"
45+
" UPDATE t SET x = CASE WHEN a THEN 1 END;\n"
46+
" UPDATE t SET y = CASE WHEN b THEN 2 ELSE 3 END;\n"
47+
"END;"
48+
)
49+
pieces = _split_top_level_statements(sql)
50+
assert len(pieces) == 1
51+
52+
53+
class TestTriggerPreambleComments:
54+
@pytest.mark.parametrize(
55+
"sql",
56+
[
57+
"CREATE/* migration v3 */TRIGGER aud AFTER INSERT ON x BEGIN UPDATE y SET v=1; END;",
58+
"CREATE -- migration v3\nTRIGGER aud AFTER INSERT ON x BEGIN UPDATE y SET v=1; END;",
59+
"CREATE TEMP /* tmp */TRIGGER aud AFTER INSERT ON x BEGIN UPDATE y SET v=1; END;",
60+
"CREATE/*A*/TEMPORARY/*B*/TRIGGER aud AFTER INSERT ON x BEGIN UPDATE y SET v=1; END;",
61+
"CREATE -- explanation\n TRIGGER aud AFTER INSERT ON x BEGIN UPDATE y SET v=1; END;",
62+
],
63+
)
64+
def test_trigger_preamble_with_comments_kept_together(self, sql: str) -> None:
65+
pieces = _split_top_level_statements(sql)
66+
assert len(pieces) == 1, f"got {len(pieces)} pieces: {pieces!r}"
67+
68+
69+
class TestRegressionsFromCycle19StillPass:
70+
def test_basic_trigger_body_kept_together(self) -> None:
71+
"""Sanity: cycle 19's basic regression test still passes."""
72+
sql = (
73+
"CREATE TRIGGER aud AFTER INSERT ON x BEGIN\n"
74+
" UPDATE y SET v=1 WHERE id=NEW.id;\n"
75+
" DELETE FROM z WHERE id=NEW.id;\n"
76+
"END;"
77+
)
78+
pieces = _split_top_level_statements(sql)
79+
assert len(pieces) == 1
80+
81+
def test_bare_begin_commit_still_split(self) -> None:
82+
"""Sanity: ordinary multi-statement batches still split."""
83+
sql = "BEGIN; INSERT INTO t VALUES (1); COMMIT;"
84+
pieces = _split_top_level_statements(sql)
85+
assert len(pieces) == 3

0 commit comments

Comments
 (0)