Skip to content

Commit eb5c174

Browse files
committed
fix: use original_query in Describe handler for RETURNING detection
In handle_describe_message, ReturningPlan was built from translated_query which has the RETURNING clause stripped (IRIS doesn't support it natively). This caused has_returning=False at Describe time → send_no_data() was sent instead of RowDescription. Clients using Extended Query Protocol (postgres.js, Drizzle ORM, Better Auth, psycopg3 with returning()) discarded all rows. Fix: use original_query for ReturningPlan in both Describe Statement and Describe Portal branches. translated_query is still used for is_select/is_dml checks (which are based on the translated form). Adds 4 e2e regression tests (prepare=True forces Extended Query Protocol). Bumps version to 1.5.5.
1 parent 0e93ad0 commit eb5c174

File tree

4 files changed

+218
-3
lines changed

4 files changed

+218
-3
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [1.5.5] - 2026-03-09
11+
12+
### Fixed
13+
- **`INSERT ... RETURNING` always empty with Extended Query Protocol (Drizzle / postgres.js / Better Auth)**: When a client uses the Extended Query Protocol (Parse → Describe → Bind → Execute), the Describe phase was incorrectly sending `NoData` instead of `RowDescription` for `INSERT ... RETURNING` statements. Root cause: `handle_describe_message` built the `ReturningPlan` from `translated_query`, which has the `RETURNING` clause stripped (because IRIS doesn't support it natively). `has_returning` was therefore always `False` at Describe time, causing `send_no_data()`. Clients like postgres.js and Drizzle ORM rely on the Describe response to decide whether to read rows at Execute time — receiving `NoData` caused them to discard all returned rows silently. Fixed by using `original_query` (the pre-translation SQL, which still contains `RETURNING`) for `ReturningPlan` detection in both the "Describe Statement" (`describe_type == "S"`) and "Describe Portal" (`describe_type == "P"`) branches of `handle_describe_message`.
14+
- **Impact**: Drizzle ORM `.returning()` returned `[]`. Better Auth failed at sign-in ("Failed to create session") because session creation uses `.returning()`. Any ORM or client using Extended Query Protocol with `RETURNING` (SQLAlchemy `returning()` with psycopg3, etc.) was affected.
15+
16+
### Added
17+
- **E2E regression tests for RETURNING via Extended Query Protocol** (`tests/e2e/test_returning_extended_protocol.py`): 4 passing tests covering `INSERT ... RETURNING *`, specific columns, multiple rows, and RowDescription presence verification via `prepare=True` (forces Extended Query Protocol). 1 xfail for `UPDATE ... RETURNING` (separate pre-existing limitation).
18+
1019
## [1.5.4] - 2026-03-06
1120

1221
### Fixed

src/iris_pgwire/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
caretdev/sqlalchemy-iris.
77
"""
88

9-
__version__ = "1.5.4"
9+
__version__ = "1.5.5"
1010
__author__ = "Thomas Dyar <thomas.dyar@intersystems.com>"
1111

1212
# Don't import server/protocol in __init__ to avoid sys.modules conflicts

src/iris_pgwire/protocol.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2930,10 +2930,14 @@ async def handle_describe_message(self, body: bytes):
29302930
# For SELECT/SHOW statements, send RowDescription with column metadata
29312931
# For DDL/DML statements, send NoData
29322932
query = stmt.get("translated_query", stmt.get("query", ""))
2933+
# Use original_query for RETURNING detection: the translation pipeline
2934+
# strips RETURNING before storing translated_query (IRIS doesn't support it
2935+
# natively), so has_returning would always be False if we use translated_query.
2936+
original_query = stmt.get("original_query", query)
29332937
query_upper = query.strip().upper()
29342938

29352939
plan = ReturningPlan.from_sql(
2936-
query,
2940+
original_query,
29372941
metadata_cache=self.iris_executor.metadata_cache,
29382942
executor=self.iris_executor,
29392943
)
@@ -3081,10 +3085,13 @@ async def handle_describe_message(self, body: bytes):
30813085

30823086
stmt = self.prepared_statements[statement_name]
30833087
query = stmt.get("translated_query", stmt.get("query", ""))
3088+
# Use original_query for RETURNING detection: translated_query has the
3089+
# RETURNING clause stripped (IRIS doesn't support it natively).
3090+
original_query = stmt.get("original_query", query)
30843091
query_upper = query.strip().upper()
30853092

30863093
plan = ReturningPlan.from_sql(
3087-
query,
3094+
original_query,
30883095
metadata_cache=self.iris_executor.metadata_cache,
30893096
executor=self.iris_executor,
30903097
)
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
"""
2+
E2E regression test: INSERT/UPDATE ... RETURNING via Extended Query Protocol.
3+
4+
Bug: In handle_describe_message (protocol.py), RETURNING detection used
5+
translated_query (which has RETURNING stripped because IRIS doesn't support it)
6+
instead of original_query. This caused has_returning=False → send_no_data()
7+
during the Describe phase. Clients like postgres.js/Drizzle ORM that use the
8+
Extended Query Protocol (Parse→Describe→Bind→Execute) then discarded all rows
9+
returned at Execute time, making .returning() always produce [].
10+
11+
Fix: Use original_query for ReturningPlan detection in both the "Describe
12+
Statement" and "Describe Portal" branches of handle_describe_message.
13+
14+
Regression anchors:
15+
- If this test fails, the original_query fix has been reverted or broken.
16+
- Uses psycopg prepare=True to force Extended Query Protocol.
17+
"""
18+
19+
import psycopg
20+
import pytest
21+
22+
23+
# ---------------------------------------------------------------------------
24+
# Fixture: autocommit connection via pgwire
25+
# ---------------------------------------------------------------------------
26+
27+
28+
@pytest.fixture
29+
def conn(pgwire_connection_params, pgwire_server):
30+
"""Autocommit psycopg connection to PGWire."""
31+
_ = pgwire_server
32+
p = pgwire_connection_params
33+
c = psycopg.connect(
34+
host=p["host"],
35+
port=p["port"],
36+
user=p["user"],
37+
password=p["password"],
38+
dbname=p["dbname"],
39+
autocommit=True,
40+
)
41+
yield c
42+
c.close()
43+
44+
45+
@pytest.fixture(autouse=True)
46+
def returning_table(conn):
47+
"""Create a simple table for RETURNING tests; drop afterwards."""
48+
with conn.cursor() as cur:
49+
cur.execute("DROP TABLE IF EXISTS e2e_returning_ext_test")
50+
cur.execute(
51+
"CREATE TABLE e2e_returning_ext_test "
52+
"(id INT PRIMARY KEY, name VARCHAR(100), status VARCHAR(20))"
53+
)
54+
yield
55+
with conn.cursor() as cur:
56+
cur.execute("DROP TABLE IF EXISTS e2e_returning_ext_test")
57+
58+
59+
# ---------------------------------------------------------------------------
60+
# Tests
61+
# ---------------------------------------------------------------------------
62+
63+
64+
class TestReturningExtendedProtocol:
65+
"""
66+
RETURNING clauses must return rows when using Extended Query Protocol.
67+
68+
psycopg uses Extended Query Protocol for parameterized queries by default
69+
(prepare=True / binary=True), which exercises Parse→Describe→Bind→Execute.
70+
"""
71+
72+
def test_insert_returning_star_extended(self, conn, returning_table):
73+
"""
74+
INSERT ... RETURNING * via Extended Query Protocol returns the inserted row.
75+
76+
Before the fix, Describe sent NoData (RETURNING stripped from translated_query)
77+
and clients discarded the rows returned at Execute time.
78+
"""
79+
with conn.cursor() as cur:
80+
cur.execute(
81+
"INSERT INTO e2e_returning_ext_test (id, name, status) "
82+
"VALUES (%s, %s, %s) RETURNING *",
83+
(1, "Alice", "active"),
84+
prepare=True,
85+
)
86+
row = cur.fetchone()
87+
88+
assert row is not None, (
89+
"INSERT ... RETURNING * returned no rows via Extended Query Protocol. "
90+
"Likely Describe sent NoData because translated_query lacks RETURNING."
91+
)
92+
assert row[0] == 1, f"Expected id=1, got id={row[0]}"
93+
assert row[1] == "Alice", f"Expected name='Alice', got '{row[1]}'"
94+
assert row[2] == "active", f"Expected status='active', got '{row[2]}'"
95+
96+
def test_insert_returning_specific_columns_extended(self, conn, returning_table):
97+
"""
98+
INSERT ... RETURNING id, name (not *) via Extended Query Protocol returns columns.
99+
"""
100+
with conn.cursor() as cur:
101+
cur.execute(
102+
"INSERT INTO e2e_returning_ext_test (id, name, status) "
103+
"VALUES (%s, %s, %s) RETURNING id, name",
104+
(2, "Bob", "pending"),
105+
prepare=True,
106+
)
107+
row = cur.fetchone()
108+
109+
assert row is not None, (
110+
"INSERT ... RETURNING id, name returned no rows via Extended Query Protocol."
111+
)
112+
assert row[0] == 2
113+
assert row[1] == "Bob"
114+
115+
@pytest.mark.xfail(
116+
reason=(
117+
"UPDATE ... RETURNING emulation via Extended Query Protocol returns 0 rows. "
118+
"RowDescription is sent correctly (our Describe fix works), but the UPDATE "
119+
"emulation path does not yet populate rows. Pre-existing limitation; "
120+
"tracked separately from the INSERT RETURNING Describe bug."
121+
),
122+
strict=True,
123+
)
124+
def test_update_returning_extended(self, conn, returning_table):
125+
"""
126+
UPDATE ... RETURNING via Extended Query Protocol returns updated row.
127+
"""
128+
with conn.cursor() as cur:
129+
# seed a row first (Simple Query Protocol ok here)
130+
cur.execute(
131+
"INSERT INTO e2e_returning_ext_test (id, name, status) VALUES (3, 'Carol', 'active')"
132+
)
133+
134+
with conn.cursor() as cur:
135+
cur.execute(
136+
"UPDATE e2e_returning_ext_test SET status = %s WHERE id = %s RETURNING id, status",
137+
("inactive", 3),
138+
prepare=True,
139+
)
140+
row = cur.fetchone()
141+
142+
assert row is not None, "UPDATE ... RETURNING returned no rows via Extended Query Protocol."
143+
assert row[0] == 3
144+
assert row[1] == "inactive"
145+
146+
def test_insert_returning_multiple_rows(self, conn, returning_table):
147+
"""
148+
Multiple INSERTs with RETURNING must each return a row.
149+
Simulates how ORMs like Drizzle issue individual inserts with prepare=True.
150+
"""
151+
inserts = [(10, "Dan", "active"), (11, "Eve", "active"), (12, "Frank", "active")]
152+
returned_ids = []
153+
154+
with conn.cursor() as cur:
155+
for row_data in inserts:
156+
cur.execute(
157+
"INSERT INTO e2e_returning_ext_test (id, name, status) "
158+
"VALUES (%s, %s, %s) RETURNING id",
159+
row_data,
160+
prepare=True,
161+
)
162+
row = cur.fetchone()
163+
assert row is not None, (
164+
f"No row returned for INSERT id={row_data[0]} via Extended Query Protocol."
165+
)
166+
returned_ids.append(row[0])
167+
168+
assert returned_ids == [10, 11, 12], (
169+
f"Expected returned ids [10, 11, 12], got {returned_ids}"
170+
)
171+
172+
def test_describe_statement_sends_row_description_not_no_data(self, conn, returning_table):
173+
"""
174+
When a prepared statement with RETURNING is Described, the server must
175+
send RowDescription (not NoData). Verified indirectly: psycopg will raise
176+
ProgrammingError / return None description if NoData was sent.
177+
178+
This is the *root cause* scenario from the bug report.
179+
"""
180+
with conn.cursor() as cur:
181+
# Prepare the statement explicitly — triggers Parse + Describe Statement
182+
cur.execute(
183+
"INSERT INTO e2e_returning_ext_test (id, name, status) "
184+
"VALUES (%s, %s, %s) RETURNING id, name, status",
185+
(20, "Grace", "active"),
186+
prepare=True,
187+
)
188+
# description is set only if RowDescription was received (not NoData)
189+
assert cur.description is not None, (
190+
"cur.description is None after INSERT ... RETURNING prepare=True. "
191+
"Describe phase sent NoData instead of RowDescription — "
192+
"original_query fix may be missing."
193+
)
194+
col_names = [d.name.lower() for d in cur.description]
195+
assert "id" in col_names, f"'id' not in description columns: {col_names}"
196+
197+
row = cur.fetchone()
198+
assert row is not None
199+
assert row[0] == 20

0 commit comments

Comments
 (0)