diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0a4b255..ca47851 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,6 +25,17 @@ jobs: enable-cache: true cache-dependency-glob: pyproject.toml - run: uv pip install --system -e ".[dev]" + + # pg_mimic develops against a commit on sqlglot's main, not a release: the + # executor fixes it needs are merged upstream and unreleased. After the + # editable install, so it wins over the floor that install pulled in. The + # floor itself (`sqlglot>=30.18.0`, what a `pip install pg-mimic` gets) + # stays covered by the `latest-release` leg of upstream-sqlglot.yml, which + # runs this same suite daily. + - run: tools/install_sqlglot.sh + env: + UV_SYSTEM_PYTHON: 1 + # -vvv names every test, --durations=0 reports every duration (not just the # slowest N) so a test that starts creeping is visible before it times out. - run: pytest -vvv --durations=0 diff --git a/.github/workflows/upstream-sqlglot.yml b/.github/workflows/upstream-sqlglot.yml index f58bcc4..df912e1 100644 --- a/.github/workflows/upstream-sqlglot.yml +++ b/.github/workflows/upstream-sqlglot.yml @@ -34,25 +34,18 @@ jobs: - run: uv pip install --system -e ".[dev]" - # After the editable install, so it wins over the pinned floor. + # After the editable install, so it wins over the floor. - name: Install the sqlglot under test + env: + UV_SYSTEM_PYTHON: 1 run: | if [ "${{ matrix.sqlglot }}" = "main" ]; then - # Cloned by hand rather than `uv pip install git+...`, because that runs - # `git submodule update --init` and sqlglot pins a submodule by SSH URL - # to a private repo (fivetran/sqlglot-integration-tests), which no - # runner can authenticate to. A plain clone leaves submodules alone, and - # that one is their integration suite -- not needed to build or import. - # - # --filter=blob:none, not --depth 1: a shallow clone carries no tags, so - # setuptools_scm cannot derive a version and produces 0.0.1.dev1, which - # then violates our own sqlglot floor. A blobless clone keeps every tag - # and is nearly as quick. - git clone --quiet --filter=blob:none https://github.com/tobymao/sqlglot "$RUNNER_TEMP/sqlglot-main" - uv pip install --system --upgrade "$RUNNER_TEMP/sqlglot-main" + # The branch tip, not tools/install_sqlglot.sh's pinned commit -- + # this leg's job is to see what is queued upstream *beyond* the pin. + tools/install_sqlglot.sh main else - uv pip install --system --upgrade sqlglot + uv pip install --upgrade sqlglot + python -c "import sqlglot; print('sqlglot', sqlglot.__version__)" fi - python -c "import sqlglot; print('sqlglot', sqlglot.__version__)" - run: pytest -vvv --durations=0 diff --git a/pyproject.toml b/pyproject.toml index 02185bf..a05ba44 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,6 +42,12 @@ dependencies = [ # ordering and the ORDER BY of a set operation -- four repairs TableSession # carried and no longer needs. Below it, each answers with the wrong rows. # tests/test_sqlglot_workarounds.py is what holds the floor to it. + # + # This is a floor, not what pg_mimic develops against: the fixes it wants next + # are merged on sqlglot's main and unreleased, and PyPI rejects a direct-URL + # dependency, so the commit lives in tools/install_sqlglot.sh instead and CI + # installs it over this. Raise this to the release that carries those fixes + # when there is one, and the script goes away. "sqlglot>=30.18.0", "scramp>=1.4.5", ] diff --git a/tests/test_sqlglot_workarounds.py b/tests/test_sqlglot_workarounds.py index 366b0c4..0051f59 100644 --- a/tests/test_sqlglot_workarounds.py +++ b/tests/test_sqlglot_workarounds.py @@ -27,6 +27,14 @@ https://github.com/jbylund/pg_mimic/issues/49, and the ones reaching users untreated are https://github.com/jbylund/pg_mimic/issues/58 +**The invariant: every workaround pg_mimic carries has a tripwire here.** Without +it this file's whole premise leaks -- a workaround with no tripwire is exactly the +thing it exists to prevent. Three were missing and are covered now: +`_make_decimal_comparisons_exact`, `_distinct_on_keys`/`_first_row_per_key`, and +the `REGEXPLIKE` patch in catalog_rewrite.py. That last one is the reason to care: +it is an `ENV.setdefault`, so the day upstream adds the function it goes dead +silently. If you add a workaround, add its tripwire in the same change. + The tests from `test_distinct_keeps_columns_that_share_a_name` down were found by `tools/fuzz`, which generates random SELECTs and compares the executor against a real PostgreSQL. Reading them as a group is the honest summary of how far the @@ -43,10 +51,23 @@ from decimal import Decimal import pytest +import sqlglot from sqlglot.executor import execute _UPSTREAM_FIXED = "sqlglot fixed this -- remove the workaround named in the test" +# v30.18.0 is the newest release and the floor, and it is the last one without the +# fix the single tripwire below carries this mark for. Every other version -- the +# main commit tools/install_sqlglot.sh pins, and every release after this one -- +# is expected to have it, so the test must pass there. A 30.19.0 that somehow +# shipped without it fails loudly, which is the news a tripwire exists to deliver. +# +# Marked conditionally rather than unmarked outright because unmarking a fix that +# is merged-but-unreleased would make the `latest-release` leg of +# upstream-sqlglot.yml permanently red -- #141's complaint with the legs swapped. +# Delete this, and the workaround the test names, when the floor moves. +_UNRELEASED = sqlglot.__version__ == "30.18.0" + def _rows(sql: str, tables: dict, schema: dict) -> list[tuple]: return [tuple(row) for row in execute(sql, schema=schema, tables=tables, dialect="postgres").rows] @@ -81,7 +102,7 @@ def test_order_by_on_a_set_operation_keeps_the_columns(): assert _rows("SELECT a FROM t UNION SELECT a FROM t ORDER BY a", *_NUMBERS) == [(1,), (2,), (3,)] -@pytest.mark.xfail(strict=True, reason=_UPSTREAM_FIXED) +@pytest.mark.xfail(_UNRELEASED, strict=True, reason=_UPSTREAM_FIXED) def test_select_distinct_ignores_the_order_by_direction(): """https://github.com/jbylund/pg_mimic/issues/49 @@ -107,6 +128,12 @@ def test_select_distinct_ignores_the_order_by_direction(): `SELECT DISTINCT b, a FROM t ORDER BY a` is wrong with no DESC in it at all. Asserted narrowly here because this file is about whether upstream has fixed it; the full matrix is in the issue. + + Fixed on sqlglot's main by + https://redirect.github.com/tobymao/sqlglot/pull/8321, which plans the DISTINCT + below the Sort so the ORDER BY survives it. Unreleased as of v30.18.0, hence + the `_UNRELEASED` condition on the mark: this passes on the pinned commit and + still xfails on the floor. """ assert _rows("SELECT DISTINCT a FROM t ORDER BY a DESC", *_NUMBERS) == [(3,), (2,), (1,)] assert _rows("SELECT DISTINCT a FROM t ORDER BY a DESC LIMIT 1", *_NUMBERS) == [(3,)] @@ -395,6 +422,64 @@ def test_full_outer_join_preserves_unmatched_rows(): assert sorted(rows, key=str) == [(1, None), (2, 2), (None, 3)] +@pytest.mark.xfail(strict=True, reason=_UPSTREAM_FIXED) +def test_a_decimal_constant_is_compared_as_numeric(): + """https://github.com/jbylund/pg_mimic/issues/49 + + Workaround: `_make_decimal_comparisons_exact` in tables.py. + + The executor evaluates a bare `9.99` as a Python float, and + `Decimal("9.99") == 9.99` is False, so a `numeric` column never matches a + decimal constant: `where total = 9.99` misses the row it should find. Postgres + types the constant as `numeric` and finds it. + + Distinct from `_cast` and test_a_decimal_cast_is_exact above, which is about an + explicit CAST. This one has no CAST in it -- the literal's own type is what is + wrong -- so the two are independently fixable and get a tripwire each. + """ + tables, schema = {"m": [{"total": Decimal("9.99")}]}, {"m": {"total": "DECIMAL(10,2)"}} + assert _rows("SELECT total FROM m WHERE total = 9.99", tables, schema) == [(Decimal("9.99"),)] + + +@pytest.mark.xfail(strict=True, reason=_UPSTREAM_FIXED) +def test_distinct_on_keeps_only_the_first_row_per_key(): + """https://github.com/jbylund/pg_mimic/issues/49 + + Workaround: `_distinct_on_keys` / `_first_row_per_key` in tables.py. + + The executor parses `DISTINCT ON` and then returns the duplicate rows anyway -- + every row, as though the clause were not there -- and it has no window functions + to rewrite it into, so pg_mimic finishes it in Python. + + The most user-visible of the untripwired workarounds: `DISTINCT ON` is + Postgres-specific syntax, so a client sending it is a client that has already + committed to Postgres. + """ + tables = {"v": [{"u": 1, "p": "a"}, {"u": 1, "p": "b"}, {"u": 2, "p": "c"}]} + schema = {"v": {"u": "INT", "p": "TEXT"}} + assert _rows("SELECT DISTINCT ON (u) u, p FROM v ORDER BY u, p", tables, schema) == [(1, "a"), (2, "c")] + + +@pytest.mark.xfail(strict=True, reason=_UPSTREAM_FIXED) +def test_regexp_like_exists(): + """https://github.com/jbylund/pg_mimic/issues/49 + + Workaround: `ENV.setdefault("REGEXPLIKE", ...)` in catalog_rewrite.py, which is + the library's only `ENV` patch. psql's own catalog SQL uses `~`, so this is on + the path of `\\d` rather than of anything a user wrote. + + `REGEXPLIKE` is absent from the executor's ENV, so Postgres's `~` and `~*` + reach the generated Python as an undefined name -- `NameError: name 'REGEXPLIKE' + is not defined`, wrapped in an ExecuteError. + + Worth a tripwire more than most: the workaround is a `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 here would notice. + """ + assert _rows("SELECT s FROM q WHERE s ~ '^Bump'", *_TEXT) == [("Bump version",)] + assert _rows("SELECT s FROM q WHERE s ~* '^bump'", *_TEXT) == [("Bump version",)] + + # Everything below was found by tools/fuzz. See the module docstring. # A NULL in here on purpose: without one, count(DISTINCT a) and count(*) are the diff --git a/tools/install_sqlglot.sh b/tools/install_sqlglot.sh new file mode 100755 index 0000000..7cf0877 --- /dev/null +++ b/tools/install_sqlglot.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# Install the sqlglot that pg_mimic develops against, which is a commit on their +# main rather than a release. +# +# Why a commit: the executor fixes pg_mimic needs are merged upstream and +# unreleased. v30.18.0 is the newest release and main is well ahead of it. The +# published package still declares `sqlglot>=30.18.0` (pyproject.toml) because +# PyPI rejects a direct-URL dependency, so this pin is a development and CI +# posture only -- never what a user installing pg-mimic gets. The floor keeps +# being exercised daily by the `latest-release` leg of upstream-sqlglot.yml. +# +# tools/install_sqlglot.sh # the pinned commit below +# tools/install_sqlglot.sh main # whatever the branch tip is now +# +# Move PINNED_REV when a fix we want lands, and delete this script entirely once +# a release carries everything (then the floor in pyproject.toml is the pin). +set -euo pipefail + +# tobymao/sqlglot main @ 2026-09-14, 43 commits past v30.18.0. Carries +# e27b7654, which plans DISTINCT before ORDER BY -- the fix that makes +# _take_result_order's sorting redundant (jbylund/sqlglot#54). +PINNED_REV="3ca824895ef423f7895fb13d72357548c0f1f367" + +rev="${1:-$PINNED_REV}" + +# uv builds a wheel from this directory, so nothing needs it afterwards. +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT +dest="$work/sqlglot" + +# Cloned by hand rather than `uv pip install git+...`, because that runs +# `git submodule update --init` and sqlglot pins a submodule by SSH URL to a +# private repo (fivetran/sqlglot-integration-tests), which no runner can +# authenticate to. A plain clone leaves submodules alone, and that one is their +# integration suite -- not needed to build or import. +# +# --filter=blob:none, not --depth 1: a shallow clone carries no tags, so +# setuptools_scm cannot derive a version and produces 0.0.1.dev1, which then +# violates our own sqlglot floor. A blobless clone keeps every tag and is nearly +# as quick. +git clone --quiet --filter=blob:none https://github.com/tobymao/sqlglot "$dest" +git -C "$dest" checkout --quiet --detach "$rev" + +# CI sets UV_SYSTEM_PYTHON=1, which is how `--system` gets in without this +# script having to know whether it is in a virtualenv. +uv pip install --quiet --upgrade "$dest" +python -c "import sqlglot; print('sqlglot', sqlglot.__version__)"