Skip to content

A UNION, EXCEPT or INTERSECT is as read-only as a SELECT - #125

Merged
jbylund merged 3 commits into
mainfrom
fix/git-sql-set-operations
Aug 17, 2026
Merged

jbylund merged 3 commits into
mainfrom
fix/git-sql-set-operations

Conversation

@jbylund

@jbylund jbylund commented Aug 17, 2026

Copy link
Copy Markdown
Owner

git_sql's read-only guard tested isinstance(expr, exp.Select), and a set operation parses to an exp.SetOperation, so every UNION, EXCEPT and INTERSECT was refused as though it were a write:

git=> SELECT subject FROM commits UNION SELECT subject FROM commits;
ERROR:  this example serves SELECT only -- a git repo is read-only

psql's own \d sends one — the publications footer is three SELECTs joined by UNION.

The guard is now (exp.Select, exp.SetOperation). Deliberately not exp.Query, which would also admit a bare exp.Subquery — (SELECT ...) LIMIT 1, whose LIMIT the executor drops, answering every row instead of one. TableSession repairs that in _flatten_parenthesized; this example has no such layer, so that shape stays refused rather than becoming silently wrong.

The second shape, which has to be refused explicitly

Letting set operations through exposed one that cannot just be allowed. sqlglot's executor answers an ORDER BY over a set operation with the columns stripped — [(), (), ()] — which is not a wrong value but a D message contradicting the RowDescription describe() already sent:

git=> SELECT name FROM branches UNION SELECT name FROM branches ORDER BY 1;
unexpected field count in "D" message

psycopg drops the connection. _reject_stripped_columns turns that into a 0A000 naming the cause. It comes out together with its test when jbylund/sqlglot#34 lands.

Tests

Both run over the wire, because in-process the stripped rows merely look odd and it is the socket that turns them into a protocol violation. Both fail without the fix. _serving() factors out the subprocess-and-connect dance the startup test already did inline.

Full suite: 888 passed, 32 xfailed. ruff check and ruff format --check clean.

Related

While measuring this I found the catalog path has none of tables.py's workarounds at all, so the same class of bug is live on \d — filed as #124. This PR does not touch that; it only stops git_sql calling a UNION a write.

jbylund and others added 3 commits August 17, 2026 07:50
git_sql's read-only guard tested `isinstance(expr, exp.Select)`, and a set
operation parses to an exp.SetOperation, so every UNION, EXCEPT and INTERSECT
was refused with "this example serves SELECT only -- a git repo is read-only".
psql's own `\d` sends one: the publications footer is three SELECTs joined by
UNION.

The guard is now (exp.Select, exp.SetOperation). Deliberately not exp.Query,
which would also admit a bare exp.Subquery -- `(SELECT ...) LIMIT 1`, whose
LIMIT the executor drops, answering every row instead of one. TableSession
repairs that in _flatten_parenthesized and this example has no such layer, so
the shape stays refused rather than silently wrong.

Letting set operations through exposed a second shape that has to be refused
explicitly. sqlglot's executor answers an ORDER BY over a set operation with
the columns stripped -- `[(), (), ()]` -- which is not a wrong value but a `D`
message contradicting the RowDescription describe() already sent, so psycopg
drops the connection with `unexpected field count in "D" message`. A 0A000 is
the honest reply until the executor keeps the columns.

Both tests run over the wire, because in-process the stripped rows merely look
odd and it is the socket that turns them into a protocol violation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI checks out a detached HEAD, so `git branch` lists nothing and the `branches`
table is empty -- `SELECT name FROM branches LIMIT 1` returned no rows and the
survives-the-error assertion failed. It also made the UNION comparison vacuous,
since two empty sets are equal.

`files` is a walk of the working tree rather than anything read out of the log,
so it has rows in any checkout. Measured against a detached-HEAD depth-1 clone
of this repo: branches 0, commits 1, files 101, commit_files 101.

The set operations are now exercised against `files` alone -- UNION and
INTERSECT with itself are every path, EXCEPT is none -- which covers all three
operators without depending on an overlap between two tables. An assertion that
the path set is non-empty keeps the rest from passing vacuously.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jbylund
jbylund merged commit 89dd90b into main Aug 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant