Pin the SQL surface curate() accepts and document the one-SELECT rule - #286
Pin the SQL surface curate() accepts and document the one-SELECT rule#286RinZ27 wants to merge 1 commit into
Conversation
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
|
👋 Hi @RinZ27 — thank you so much for your first contribution to HFlow! A maintainer will review your pull request as soon as possible. In the meantime:
💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game. We are excited to have you here and appreciate your help making the project better! 🙌 |
|
Sorry, closing this one. #279 is assigned and #285 has been open on it since 10:20, so first-PR-wins applies. Timing, nothing about your work. For next time: comment on an issue before you start and I'll assign it, and check whether it already has an assignee or a linked PR. An issue with an assignee is taken, everything else is fair game. The starter pool is picked clean this morning, more go up regularly. If you'd rather not wait, the most useful thing anyone can do here is run HFlow against a real corpus, Egocentric-10K or Egocentric-100K on HF, and report what breaks, what's slow, what's awkward. See #287. |
Summary
Pins which SQL forms
_reject_non_single_selectaccepts and refuses, and documents the one-SELECT rule indocs/CATALOG.md.Why
After #271 landed the guard, the accepted SQL surface changed in both directions but nothing wrote it down.
PIVOTis now refused (DuckDB rewrites it to[CREATE, SELECT], so the count check catches it — arguably correct rather than merely strict).DESCRIBE SELECT ...andSUMMARIZE SELECT ...moved the other way: both report asSELECTand now pass the guard, where previously they were syntax errors inside the oldCOPY (...)wrapper.Without a pin, someone will eventually "relax" the count check to let
PIVOTthrough without realising theCREATEit admits is the thing the guard exists to stop. AndDESCRIBE/SUMMARIZEworking by accident means nothing would notice if a future DuckDB reclassified them.Decision on DESCRIBE and SUMMARIZE: both are accepted incidentally. The test records that fact and names them as not a promised surface, matching what the docs paragraph now says. If a future DuckDB version reclassifies either one, the test goes red and forces a deliberate choice rather than silent breakage.
Validation
Checklist
uv run ruff check --fix,uv run ruff format, anduv run ty check.Closes #279