Skip to content

Develop against a commit on sqlglot's main, not its last release - #143

Open
jbylund wants to merge 2 commits into
mainfrom
develop-against-sqlglot-main
Open

jbylund wants to merge 2 commits into
mainfrom
develop-against-sqlglot-main

Conversation

@jbylund

@jbylund jbylund commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner

The executor fixes pg_mimic wants next are merged upstream and unreleased. v30.18.0 is still the newest release and main is 43 commits past it, carrying 8321 (plan DISTINCT below the Sort so the ORDER BY survives), 8319 and 8304. This develops against that commit.

Where the pin lives, and why not in dependencies

PyPI rejects a direct-URL dependency, so the commit cannot go in dependencies. It lives in tools/install_sqlglot.sh, which CI runs after the editable install so it wins over the floor that install pulled in.

The floor stays sqlglot>=30.18.0 — what a pip install pg-mimic actually gets — and stays exercised daily by the latest-release leg of upstream-sqlglot.yml, so flipping tests.yml to the pin costs no floor coverage. upstream-sqlglot.yml's main leg now calls the same script with main for the branch tip, so the clone's two non-obvious details live in one place: blobless rather than shallow, so setuptools_scm can still see tags and derive a version that satisfies our own floor; and hand-cloned, because uv pip install git+... runs git submodule update --init and sqlglot pins a submodule by SSH URL to a private repo no runner can authenticate to.

The one tripwire this fixes is marked conditionally, not unmarked

8321 makes test_select_distinct_ignores_the_order_by_direction XPASS on the pin, and a strict XPASS is a build failure. But the bug is still live on the floor, so unmarking it outright would turn the latest-release leg permanently red — #141's complaint with the legs swapped. Conditional on the version, it passes on the pin and still strict-xfails on the floor.

Verified both ways:

sqlglot result
pinned 3ca8248 (30.18.1.dev43) 964 passed, 19 xfailed
floor 30.18.0 963 passed, 20 xfailed

As a side effect this should take the main leg of upstream-sqlglot.yml green, since the tree now matches what main answers — which is the half of #141 that made the job's run history unreadable.

What this does not delete

Exactly one workaround becomes dead, and only on the pin: _sorted_rows and the sort keys in tables.py. It stays, because on the floor deleting it would answer SELECT DISTINCT ... ORDER BY ... DESC in the wrong order for anyone installing from PyPI. Queued for the floor bump.

With a caveat recorded on #49 for whoever does that bump: only the sorting is dead. _take_result_order also rejects SELECT DISTINCT a FROM t ORDER BY b, which Postgres rejects (42P10) and sqlglot, SQLite 3.53.4 and DuckDB 1.5.5 all answer — arbitrarily, the result flipping with physical row order in all three, because after DISTINCT a collapses two rows with the same a and different b there is no single b left to sort by. Postgres is the outlier in enforcing it, but that check is Postgres fidelity rather than a workaround, and has to survive the deletion. It has no test today.

The other 19 tripwires still xfail on main: INTERVAL, the exact-decimal cast, TABLESAMPLE, VALUES, rounding, duplicate column names, LIKE escapes, ltrim/rtrim, EXTRACT(dow) and the rest.

Issues

No user-visible behaviour changes, so no CHANGELOG entry. Updated out of band:
#123 narrowed to its _cast item (its _rewrite_not_in items are fixed, by that
workaround's deletion in b53be1a), and #49 has the new status for the SELECT DISTINCT row plus the caveat above.

Second commit: every workaround now has a tripwire

Counting what is actually left turned up three workarounds with no strict-xfail tripwire watching them, so nothing would have announced the day upstream made them deletable — the precise failure this file exists to prevent, and which it already records having happened twice (#50). Added one each for _make_decimal_comparisons_exact, _distinct_on_keys/_first_row_per_key, and the REGEXPLIKE patch in catalog_rewrite.py.

REGEXPLIKE is the one that most needed covering: it is an ENV.setdefault, so the day upstream adds the function ours goes dead silently — no error, no wrong answer, just code that never runs again. Nothing else in the suite would have noticed.

All three expectations were verified against PostgreSQL 18.6 rather than asserted from memory, per the file's own premise. All three xfail strictly on both the pin and the floor: 22 xfailed on the pin, 23 on the floor. The module docstring now states the invariant, so the next workaround added without one is a review comment rather than a discovery years later.

For the record, the workaround count this leaves: six live in the library (_cast, _make_decimal_comparisons_exact, DISTINCT ON, canonicalize_table_aliases, _flatten_parenthesized, REGEXPLIKE), one required only on the floor (_sorted_rows), and two in examples/git_sql.py (the clock, and INTERVAL). The 19 tripwires still red are mostly not workarounds — 13 of them are untreated gaps that reach users (#58).

The executor fixes pg_mimic wants next are merged upstream and unreleased:
v30.18.0 is the newest release and main is 43 commits past it, carrying 8321
(plan DISTINCT below the Sort so the ORDER BY survives), 8319 and 8304.

PyPI rejects a direct-URL dependency, so the commit cannot live in
`dependencies`. It goes in tools/install_sqlglot.sh instead, which CI runs after
the editable install so it wins over the floor that install pulled in. The floor
stays `sqlglot>=30.18.0` -- what a `pip install pg-mimic` actually gets -- and
stays exercised daily by the `latest-release` leg of upstream-sqlglot.yml.

That leg is also why the one tripwire this fixes is marked conditionally rather
than unmarked. 8321 makes `test_select_distinct_ignores_the_order_by_direction`
XPASS on the pin, and strict XPASS is a build failure -- but the bug is still
live on the floor, so unmarking it would turn the `latest-release` leg
permanently red, which is #141's complaint with the legs swapped. Conditional on
the version, it passes on the pin and still xfails on the floor. Verified both
ways: 964 passed / 19 xfailed on the pin, 963 / 20 on 30.18.0.

The workaround it names stays for now. `_sorted_rows` is dead on the pin but not
on the floor, where deleting it would answer `SELECT DISTINCT ... ORDER BY DESC`
in the wrong order for anyone installing from PyPI. Queued for the floor bump,
with a caveat recorded on #49: only the sorting is dead. `_take_result_order`
also rejects `SELECT DISTINCT a FROM t ORDER BY b`, which Postgres rejects and
sqlglot, SQLite and DuckDB all answer -- arbitrarily, the result flipping with
physical row order in all three. That check is Postgres fidelity rather than a
workaround, and has to survive the deletion.

upstream-sqlglot.yml's main leg now calls the same script, passing `main` for
the branch tip, so the clone's two non-obvious details (blobless not shallow, so
setuptools_scm can still see tags; hand-cloned because `uv pip install git+...`
runs `git submodule update --init` and sqlglot pins a submodule by SSH URL to a
private repo) live in one place.
Three of the seven workarounds pg_mimic carries had no strict-xfail tripwire, so
nothing would have announced the day upstream made them deletable -- which is the
precise failure this file exists to prevent, and which it already records having
happened twice (#50).

- `_make_decimal_comparisons_exact`: a bare `9.99` is a Python float, and
  `Decimal("9.99") == 9.99` is False, so `where total = 9.99` misses the row.
  Separate from the `_cast` tripwire above it, which needs an explicit CAST --
  independently fixable, so one tripwire each.
- `_distinct_on_keys` / `_first_row_per_key`: the executor parses `DISTINCT ON`
  and returns the duplicates anyway. The most user-visible of the three, since
  `DISTINCT ON` is Postgres-specific syntax and a client sending it has already
  committed to Postgres.
- the `REGEXPLIKE` patch in catalog_rewrite.py, the library's only `ENV` patch,
  on the path of psql's `\d` rather than of anything a user wrote. This is the
  one that most needed covering: it is an `ENV.setdefault`, so the day upstream
  adds the function ours goes dead *silently* -- no error, no wrong answer, just
  code that never runs again.

All three expectations verified against PostgreSQL 18.6 rather than asserted from
memory, per this file's premise. All three xfail strictly on both the pinned
commit and the 30.18.0 floor; 22 xfailed on the pin, 23 on the floor.

The module docstring now states the invariant, so the next workaround added
without one is a review comment rather than a discovery years later.

This branch has not been deployed

No deployments
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