Skip to content

The curation gate parses every query twice, and the second parse refuses nothing #481

Description

@kstonekuan

reject_non_single_select runs three checks in order (src/hflow/curation.py:604):

  1. exactly one statement, of type SELECT (:637)
  2. the leading keyword is not pragma / describe / show / summarize (:643)
  3. the SQL parses inside SELECT * FROM (<sql>), the shape preview interpolates (:651)

The third one refuses nothing that the first two have not already refused.

Measured

Deleting the whole third block leaves every curation test green:

### wrapper parse removed
    100 passed in 6.24s
    (nothing noticed)

So I went looking for an input it catches on its own: one that parses standalone as a single StatementType.SELECT, is not headed by one of the four keywords, and still fails to parse inside the wrapper. Across 27 shapes there is none. Every case where the wrapped parse fails, something earlier has already refused it:

sql standalone wrapped
PRAGMA database_list 1xSELECT ERR (already refused at step 2)
(PRAGMA database_list) ERR ParserException ERR (already refused at step 1)
COPY (SELECT 1) TO 'x.csv' 1xCOPY ERR (already refused at step 1)
SELECT 1 /* unterminated ERR ParserException ERR (already refused at step 1)
everything else tried 1xSELECT parses

Shapes tried that all parse both ways: SELECT 1;, SELECT 1;;, trailing line comment, ORDER BY, LIMIT, UNION, UNION ALL ... ORDER BY ... LIMIT, CTE, FROM episodes, VALUES, (SELECT 1), (DESCRIBE SELECT 1), SELECT * FROM (SHOW TABLES), SELECT * FROM pragma_version(), TABLE episodes, * EXCLUDE, QUALIFY, unnest, UNPIVOT, GROUP BY ALL, SELECT 1 UNION (SELECT 2).

What it costs

It parses every query a second time, and its only demonstrated behaviour is almost refusing valid SQL. Two workarounds exist purely to stop that: the appended \n at :651, so a trailing -- comment cannot swallow the wrapper's ), and the .rstrip(';'), so SELECT 1; survives. Both are load-bearing only for the block itself. Neither is needed once it goes.

That is the same failure direction as the regression this gate already had once: refusing legal read-only SELECTs.

What to do

Delete the block at :647-659 and the wrapped_input line, keeping steps 1 and 2. Then delete whichever of the two supporting tests no longer describe anything real, and keep the ones that still pin behaviour:

  • test_reject_non_single_select_accepts_trailing_line_comment_without_newline keeps its assertion but its comment stops being true, so rewrite the comment.
  • test_reject_non_single_select_accepts_a_trailing_semicolon stays as-is; the behaviour it pins must survive.

If you find an input the wrapper parse refuses correctly, say what it is and close this instead. That is the more useful outcome and I would rather be wrong here.

Definition of done

  1. The wrapper parse is gone, and PRAGMA/DESCRIBE/SHOW/SUMMARIZE are still refused by both /curation/preview and /curation/pin with the same message.
  2. FROM-first, parenthesized, VALUES, trailing-semicolon and trailing-comment queries are all still accepted.
  3. Mutation: delete the leading-keyword refusal, confirm a test goes red. It is now the only thing standing between the endpoints and what they advertise.
  4. Both suites green, including packages/hflow-server.

Validation

uv sync --locked --all-extras
uv run ruff check
uv run ruff format --check
uv run ty check
uv run pytest -q
cd packages/hflow-server && uv run --project . pytest -q

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions