From 9d31ad8d8ee2c250fc0dbb4dbc2735c7846031e4 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Fri, 7 Aug 2026 00:16:51 -0400 Subject: [PATCH 1/2] fix(trino): strip trailing semicolon on unlimited query path Trino statement endpoint is stricter about bare terminators; strip on unlimited execute to match limited wrap and client CLI behavior. --- core/wren/src/wren/connector/trino.py | 8 ++-- .../unit/test_trino_semicolon_unlimited.py | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 core/wren/tests/unit/test_trino_semicolon_unlimited.py diff --git a/core/wren/src/wren/connector/trino.py b/core/wren/src/wren/connector/trino.py index 1cb08cdf18..e511430c1c 100644 --- a/core/wren/src/wren/connector/trino.py +++ b/core/wren/src/wren/connector/trino.py @@ -485,10 +485,12 @@ def query(self, sql: str, limit: int | None = None) -> pa.Table: limit = coerce_limit(limit) trino = _import_trino() + # Strip terminating `;` for unlimited execute too — Trino's statement + # path is stricter than most engines about bare terminators (CLI also + # strips client-side). Limited path already strips inside the wrap. + sql = strip_trailing_semicolon(sql) if limit is not None: - sql = ( - f"SELECT * FROM ({strip_trailing_semicolon(sql)}) AS _sub LIMIT {limit}" - ) + sql = f"SELECT * FROM ({sql}) AS _sub LIMIT {limit}" try: with contextlib.closing(self.connection.cursor()) as cursor: cursor.execute(sql) diff --git a/core/wren/tests/unit/test_trino_semicolon_unlimited.py b/core/wren/tests/unit/test_trino_semicolon_unlimited.py new file mode 100644 index 0000000000..9cf77df5c4 --- /dev/null +++ b/core/wren/tests/unit/test_trino_semicolon_unlimited.py @@ -0,0 +1,42 @@ +"""Trino unlimited query strips trailing semicolons before execute.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pyarrow as pa +import pytest + + +@pytest.fixture +def connector(): + with patch("wren.connector.trino._import_trino") as imp: + mod = MagicMock() + imp.return_value = mod + from wren.connector.trino import TrinoConnector + + c = TrinoConnector.__new__(TrinoConnector) + c.connection = MagicMock() + c._closed = False + yield c, mod + + +def test_query_without_limit_strips_trailing_semicolon(connector) -> None: + c, _mod = connector + cursor = MagicMock() + c.connection.cursor.return_value = cursor + # _build_trino_arrow_table path — mock fetch + with patch("wren.connector.trino._build_trino_arrow_table", return_value=pa.table({"x": [1]})): + c.query("SELECT 1;") + cursor.execute.assert_called_once_with("SELECT 1") + + +def test_query_with_limit_strips_inside_wrap(connector) -> None: + c, _mod = connector + cursor = MagicMock() + c.connection.cursor.return_value = cursor + with patch("wren.connector.trino._build_trino_arrow_table", return_value=pa.table({"x": [1]})): + c.query("SELECT 1;", limit=5) + sent = cursor.execute.call_args[0][0] + assert "SELECT 1;" not in sent + assert "LIMIT 5" in sent From b79844df9471655d8fe5f097f5f7a6faa3888ab7 Mon Sep 17 00:00:00 2001 From: Bartok9 <259807879+Bartok9@users.noreply.github.com> Date: Fri, 7 Aug 2026 02:49:42 -0400 Subject: [PATCH 2/2] refactor(trino): strip trailing semicolon on unlimited query path Frame as connector consistency with merged mysql/mssql precedents; drop unsubstantiated Trino-strictness claim from the comment. --- core/wren/src/wren/connector/trino.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/wren/src/wren/connector/trino.py b/core/wren/src/wren/connector/trino.py index e511430c1c..8afbf2eeba 100644 --- a/core/wren/src/wren/connector/trino.py +++ b/core/wren/src/wren/connector/trino.py @@ -485,9 +485,9 @@ def query(self, sql: str, limit: int | None = None) -> pa.Table: limit = coerce_limit(limit) trino = _import_trino() - # Strip terminating `;` for unlimited execute too — Trino's statement - # path is stricter than most engines about bare terminators (CLI also - # strips client-side). Limited path already strips inside the wrap. + # Align unlimited execute with other connectors (mysql/mssql/etc.): + # strip a terminating `;` before send. Limited composition still needs + # a clean inner SQL so `;` cannot break the subquery wrap. sql = strip_trailing_semicolon(sql) if limit is not None: sql = f"SELECT * FROM ({sql}) AS _sub LIMIT {limit}"