Repository navigation
Move the sqlglot floor to 30.18.0 and delete what it makes dead - #142
Merged
Merged
Conversation
Closes #109, closes #72. Strikes four rows off #49 and seven items off #58; #123 keeps only item 3. sqlglot v30.18.0 fixes thirteen of the executor bugs the strict-xfail tripwires in tests/test_sqlglot_workarounds.py were watching for. Four of them were repairs in tables.py, so the repairs go: - _rewrite_not_in, with _anti_join and _null_count -- 8190, 8236 - _rewrite_null_ordering -- 8158 - the nested-OFFSET arm of _reject_silently_ignored, and the offset arm of rows_sliced_here -- 8219. A nested OFFSET is answered now rather than refused. - the SetOperation arm of _take_result_order -- 8214 - ENV.setdefault("DPIPE", ...) in examples/git_sql.py, and a stale DPIPE comment in catalog.py -- 8146 Net 150 lines out of tables.py. No answer changes; there is less pg_mimic between the query and them. _take_result_order and _sorted_rows survive, for a reason #109 recorded wrongly. It expected the SELECT DISTINCT half to be a pg_mimic layering artifact -- keeping the IS NULL key _rewrite_null_ordering added out of DISTINCT's dedup -- and so to become deletable with it. It is a live upstream defect instead: the executor discards a SELECT DISTINCT's ORDER BY and sorts ascending by the select list, so a LIMIT returns the wrong rows and `SELECT DISTINCT b, a FROM t ORDER BY a` is wrong with no DESC in it. Reported as jbylund/sqlglot#54, and it now has its own tripwire rather than sharing one with the set operation that got fixed -- a single assertion over two causes cannot XPASS, which is the #107 lesson again. Confirmed with tools/fuzz against PostgreSQL 18: 142 divergence buckets on a C-locale database, against 193 on the old floor and the 144 #109 predicted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #109, closes #72. Strikes four rows off #49 and seven items off #58; #123 keeps only item 3.
sqlglot v30.18.0 shipped on 2026-09-03 and thirteen of the strict-xfail tripwires in
test_sqlglot_workarounds.pyXPASSed on the version bump — which is the whole point of that file, working for the first time at scale. Four were repairs intables.py, so the repairs go.Deleted
_rewrite_not_in, with_anti_joinand_null_count_rewrite_null_orderingOFFSETarm of_reject_silently_ignored, and theoffsetarm ofrows_sliced_hereSetOperationarm of_take_result_orderENV.setdefault("DPIPE", ...)inexamples/git_sql.py, plus a staleDPIPEcomment incatalog.pyNet 150 lines out of
tables.py. No answer changes — every row a client gets is the same one, with less pg_mimic between the query and it. The one behaviour change is a widening: a nestedOFFSET(SELECT ... FROM (SELECT ... OFFSET 2) AS page, or in a CTE or aUNIONbranch) is now answered rather than refused with0A000, because the executor applies one wherever it appears._take_result_ordersurvives, and #109 said why wronglyThis is the part worth reviewing. #109's checklist predicted the
SELECT DISTINCThalf of_take_result_orderwas a pg_mimic layering artifact — there to keep theIS NULLkey_rewrite_null_orderingspliced in out ofDISTINCT's dedup — and would therefore become deletable along with it.It is a live upstream defect instead. With
_rewrite_null_orderinggone, the executor still discards aSELECT DISTINCT'sORDER BYentirely and sorts ascending by the select list:SELECT DISTINCT a FROM t ORDER BY a DESC[(1,), (2,), (3,)][(3,), (2,), (1,)]SELECT DISTINCT a FROM t ORDER BY a DESC LIMIT 1[(1,)][(3,)]SELECT DISTINCT b, a FROM t ORDER BY a(b, a)aThe
LIMITrow is the severe one — wrong rows, not a wrong order — and the third has noDESCin it at all, so it is not a direction bug. Cause is in the planner: theDISTINCTaggregate is stacked above theSortstep and then re-sorts, because it groups by sorting.GROUP BYbuilds the same two steps the other way round and is correct. Reported as jbylund/sqlglot#54 with the plan dumps.So the function stays, narrowed to the
SELECT DISTINCTcase, and gets its own tripwire instead of sharing one with the set operation that got fixed. That sharing is exactly why this went unnoticed: a single assertion over two causes cannot XPASS when only one is fixed — the #107 lesson, and the same shape as thebtrim/ltrimsplit.Tests
963 passed, 20 xfailed (from 961/33). Thirteen marks came off; the tests stay as plain assertions, which is what pins the floor.
New:
test_select_distinct_ignores_the_order_by_direction— the tripwire for executor: SELECT DISTINCT discards the ORDER BY and sorts by the select list [CLAUDE] sqlglot#54, asserting theLIMITcase too.test_an_offset_inside_a_subquery_pages_too— replacestest_an_offset_the_result_cannot_carry_is_refused, which asserted a refusal that is no longer correct.test_not_in_a_subquery_with_a_null_left_operand— The exact-decimal_castmakes aDecimal/floatpair Python won't divide #123 items 1 and 2, againstTableSessionrather than against sqlglot. This is the gap that let The exact-decimal_castmakes aDecimal/floatpair Python won't divide #123 go unnoticed: the tripwires next door test upstream, so a workaround that was itself wrong had nothing failing. Both are fixed by the deletion, and now stay fixed.test_a_plain_row_window_is_left_on_the_query— asserts the plan, not the rows, since both spellings answer correctly either way. A query the executor can window itself must reach it with the window attached.Renamed
test_is_not_null_answers_a_booleantotest_an_operator_after_a_case_binds_to_the_whole_case, per #109:IS NOT NULLwas never the defect. #8176 fixed an unparenthesizedCASEin the generated Python, so any operator after aCASEbound wrong;IS NOT NULLwas the vehicle the fuzzer arrived in.Confirmed with
tools/fuzz --target mimic --count 4000against PostgreSQL 18: 142 divergence buckets on a C-locale database, against 193 on the old floor and the 144 #109 predicted for this change. (153 on anen_US.UTF-8database — the extra 11 are collation findings, which is what the--locale=Cnote intools/fuzz/README.mdis for.)ruff checkandruff format --checkclean.Not in scope
Three
#49rows are untouched and still needed:canonicalize_table_aliases,_flatten_parenthesized(a parenthesized top-level query's own row window is still ignored —(SELECT a FROM t) LIMIT 1returns every row), and theTABLESAMPLErefusal.DISTINCT ONis still finished in Python and still refused inside a subquery, since the executor ignores it entirely.Nothing else was deleted on the strength of #8190 alone, per #109's warning: jbylund/sqlglot#35 is unchanged by this release, so the unsound
unnest_subqueriesrewrites are still out there.#123 item 3 (
CAST(1 AS DECIMAL) / 0.5raising onDecimal / float) is untouched — it is #58 item 14 reached through our own_cast, and nothing upstream addressed it.The daily
Upstream sqlglotjob did not announce this release, and that is filed separately as #141: the job is fine and went red on exactly the right run, but themainleg is permanently red by design, so the workflow's conclusion has beenfailureevery day and the one actionable transition was invisible.