From 57b098d28a616356d3228a84e68f73cb3f6e3b01 Mon Sep 17 00:00:00 2001 From: RinZ27 <222222878+RinZ27@users.noreply.github.com> Date: Sun, 30 Aug 2026 18:35:19 +0700 Subject: [PATCH] Pin the SQL surface curate() accepts and document the one-SELECT rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a parametrized test covering the edge cases from issue #279: PIVOT (refused — DuckDB expands it to CREATE + SELECT), DESCRIBE/SUMMARIZE (accepted incidentally as SELECT), TABLE/VALUES/WITH...SELECT (accepted), and DDL/DML/multi-statement strings (refused). Add a paragraph to docs/CATALOG.md stating the rule where curation SQL is described. Closes #279 --- docs/CATALOG.md | 9 +++ tests/test_catalog_curation.py | 105 +++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/docs/CATALOG.md b/docs/CATALOG.md index 0ee207c..4ada20e 100644 --- a/docs/CATALOG.md +++ b/docs/CATALOG.md @@ -186,6 +186,15 @@ posture a service uses for tenant-supplied SQL ([docs/HOSTING.md](./HOSTING.md#trust-model)). The default stays unrestricted for your own exploration. +Curation SQL must be exactly one `SELECT` statement. `curate()` parses the +input with DuckDB's `extract_statements` before executing anything, and +refuses multi-statement strings, DDL, DML, and queries that DuckDB expands +internally (notably `PIVOT`, which DuckDB rewrites to a `CREATE` followed by +a `SELECT`). `WITH ... SELECT`, `TABLE`, and `VALUES` all classify as a +single `SELECT` and are accepted. `DESCRIBE SELECT ...` and +`SUMMARIZE SELECT ...` also pass the guard because DuckDB reports them as +`SELECT`; they work today but are not a promised surface. + ### The view surface `hflow.open_catalog_connection(catalog_root)` returns a DuckDB connection with diff --git a/tests/test_catalog_curation.py b/tests/test_catalog_curation.py index 69aa964..b46019b 100644 --- a/tests/test_catalog_curation.py +++ b/tests/test_catalog_curation.py @@ -909,6 +909,111 @@ def test_stage_manifest_and_count_rejects_non_single_select( connection.close() +# -- Pin the SQL surface that _reject_non_single_select accepts (#279) -------- +# +# DuckDB's extract_statements classifies each input and the guard requires +# exactly one statement whose type is SELECT. Several query forms land in +# surprising buckets: +# +# PIVOT → DuckDB rewrites it to [CREATE, SELECT], so the *count* check +# catches it. That is arguably correct rather than merely strict. +# DESCRIBE / SUMMARIZE → both report as SELECT, so they pass the guard. +# Before #271 they were syntax errors inside the COPY (...) wrapper. +# TABLE / VALUES → report as SELECT, so they pass. +# +# Whether DESCRIBE and SUMMARIZE *should* be accepted is a policy decision. +# As of this writing (main @ 48d53cf, #271 merged) they are accepted, and +# this table records that fact so the next reader sees a deliberate pin +# rather than an accident. + +_SQL_SURFACE_CASES = [ + # (sql, expected_refused) + pytest.param( + "SELECT 1", + False, + id="plain-select", + ), + pytest.param( + "WITH cte AS (SELECT 1 AS x) SELECT * FROM cte", + False, + id="with-select", + ), + pytest.param( + "TABLE episodes", + False, + id="table-shorthand", + ), + pytest.param( + "VALUES (1), (2)", + False, + id="values-expression", + ), + pytest.param( + "DESCRIBE SELECT 1", + False, + id="describe-select-accepted-incidentally", + ), + pytest.param( + "SUMMARIZE SELECT 1", + False, + id="summarize-select-accepted-incidentally", + ), + # -- Refused --- + pytest.param( + # DuckDB expands PIVOT to two statements: [CREATE, SELECT]. + # The count check refuses it, which is correct: the CREATE is real. + "PIVOT episodes ON status USING count(*)", + True, + id="pivot-expands-to-create-plus-select", + ), + pytest.param( + "CREATE TABLE t(x INT)", + True, + id="ddl-only", + ), + pytest.param( + "INSERT INTO episodes VALUES (1)", + True, + id="dml-insert", + ), + pytest.param( + "DELETE FROM episodes", + True, + id="dml-delete", + ), + pytest.param( + "SELECT 1; SELECT 2", + True, + id="two-selects", + ), +] + + +@pytest.mark.parametrize("sql,expected_refused", _SQL_SURFACE_CASES) +def test_reject_non_single_select_sql_surface( + sql: str, + expected_refused: bool, +) -> None: + """Pin which SQL forms the one-SELECT guard accepts and refuses. + + See issue #279 for the full rationale. The guard is + ``_reject_non_single_select`` in ``src/hflow/curation.py``. It uses + DuckDB's ``extract_statements`` to parse without executing, requires + exactly one statement, and requires its type to be SELECT. + + This test calls the guard directly on strings — no catalog, no + connection, no files — because ``extract_statements`` is a pure parse + over the input text. + """ + from hflow.curation import _reject_non_single_select + + if expected_refused: + with pytest.raises(ValueError, match="exactly one SELECT"): + _reject_non_single_select(sql) + else: + _reject_non_single_select(sql) # must not raise + + def test_write_parquet_and_copy_format_parquet_are_byte_identical( tmp_path: Path, ) -> None: