Skip to content

Commit 03bd1b3

Browse files
authored
Merge pull request #219 from Maxteabag/fix-sqlite-returning-147
Support RETURNING clause in DML statements
2 parents 3099697 + 2c8acaf commit 03bd1b3

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

sqlit/domains/connections/providers/sqlite/adapter.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,9 @@ def execute_query(self, conn: Any, query: str, max_rows: int | None = None) -> t
220220
else:
221221
rows = cursor.fetchall()
222222
truncated = False
223+
# DML with RETURNING produces a result set but also writes — persist it.
224+
if conn.in_transaction:
225+
conn.commit()
223226
return columns, [tuple(row) for row in rows], truncated
224227
return [], [], False
225228

sqlit/domains/query/app/query_service.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
# Query types that return result sets (SELECT-like queries)
1919
SELECT_KEYWORDS = frozenset(["SELECT", "WITH", "SHOW", "DESCRIBE", "EXPLAIN", "PRAGMA"])
2020

21+
# DML statements that may carry a RETURNING clause — when present they produce a result set.
22+
_DML_KEYWORDS = frozenset(["INSERT", "UPDATE", "DELETE", "MERGE"])
23+
_RETURNING_RE = re.compile(r"(?is)\bRETURNING\b\s+\S")
24+
2125
# Regex for parsing USE database statements
2226
# Matches: USE dbname, USE [dbname], USE `dbname`, USE "dbname"
2327
_USE_PATTERN = re.compile(
@@ -93,7 +97,12 @@ def classify(self, query: str) -> QueryKind:
9397
if non_comment_lines:
9498
first_line = non_comment_lines[0].upper()
9599
first_word = first_line.split()[0] if first_line else ""
96-
return QueryKind.RETURNS_ROWS if first_word in SELECT_KEYWORDS else QueryKind.NON_QUERY
100+
if first_word in SELECT_KEYWORDS:
101+
return QueryKind.RETURNS_ROWS
102+
# DML with a RETURNING clause produces a result set too.
103+
if first_word in _DML_KEYWORDS and _RETURNING_RE.search(stmt):
104+
return QueryKind.RETURNS_ROWS
105+
return QueryKind.NON_QUERY
97106

98107
return QueryKind.NON_QUERY
99108

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
"""Regression tests for issue #147: SQLite UPDATE ... RETURNING crashes on commit."""
2+
3+
from __future__ import annotations
4+
5+
import sqlite3
6+
from pathlib import Path
7+
8+
import pytest
9+
10+
from sqlit.domains.connections.providers.sqlite.adapter import SQLiteAdapter
11+
from sqlit.domains.query.app.query_service import KeywordQueryAnalyzer, QueryKind
12+
13+
14+
@pytest.fixture
15+
def jobs_db(tmp_path: Path) -> Path:
16+
"""A tiny SQLite DB with a `jobs` table for RETURNING tests."""
17+
db = tmp_path / "jobs.db"
18+
conn = sqlite3.connect(str(db))
19+
conn.execute("CREATE TABLE jobs (id INTEGER PRIMARY KEY, status TEXT)")
20+
conn.executemany("INSERT INTO jobs (id, status) VALUES (?, ?)", [(1, "new"), (2, "new")])
21+
conn.commit()
22+
conn.close()
23+
return db
24+
25+
26+
def test_classifier_recognizes_update_returning_as_returns_rows():
27+
"""`UPDATE ... RETURNING` produces a result set, so the analyzer must classify it as RETURNS_ROWS."""
28+
analyzer = KeywordQueryAnalyzer()
29+
sql = "UPDATE jobs SET status = status WHERE id = 1 RETURNING id"
30+
assert analyzer.classify(sql) == QueryKind.RETURNS_ROWS
31+
32+
33+
def test_classifier_recognizes_insert_returning_as_returns_rows():
34+
analyzer = KeywordQueryAnalyzer()
35+
sql = "INSERT INTO jobs (id, status) VALUES (3, 'new') RETURNING id"
36+
assert analyzer.classify(sql) == QueryKind.RETURNS_ROWS
37+
38+
39+
def test_classifier_recognizes_delete_returning_as_returns_rows():
40+
analyzer = KeywordQueryAnalyzer()
41+
sql = "DELETE FROM jobs WHERE id = 1 RETURNING id"
42+
assert analyzer.classify(sql) == QueryKind.RETURNS_ROWS
43+
44+
45+
def test_classifier_plain_update_is_non_query():
46+
"""Plain DML without RETURNING must still be NON_QUERY (sanity check we don't over-correct)."""
47+
analyzer = KeywordQueryAnalyzer()
48+
assert analyzer.classify("UPDATE jobs SET status = 'done'") == QueryKind.NON_QUERY
49+
50+
51+
def test_sqlite_execute_query_runs_update_returning_and_persists(jobs_db: Path):
52+
"""UPDATE ... RETURNING via execute_query must return the row AND persist the change."""
53+
adapter = SQLiteAdapter()
54+
conn = sqlite3.connect(str(jobs_db))
55+
try:
56+
columns, rows, _ = adapter.execute_query(
57+
conn,
58+
"UPDATE jobs SET status = 'done' WHERE id = 1 RETURNING id, status",
59+
)
60+
assert columns == ["id", "status"]
61+
assert rows == [(1, "done")]
62+
finally:
63+
conn.close()
64+
65+
# Verify the write was actually committed by opening a fresh connection.
66+
verify = sqlite3.connect(str(jobs_db))
67+
try:
68+
result = verify.execute("SELECT status FROM jobs WHERE id = 1").fetchone()
69+
assert result == ("done",), f"UPDATE ... RETURNING did not persist; got {result!r}"
70+
finally:
71+
verify.close()

0 commit comments

Comments
 (0)