Skip to content

test: port analyze_differential to pytest (#432) - #1161

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:test/432-analyze-differential
Sep 19, 2026
Merged

jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:test/432-analyze-differential

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Port of analyze_differential.sh (#432). 20 names, missing: 0, no extras.

literal matches: 18 | template matches: 2 | missing: 0
VERDICT: every bash property is covered

Why this suite cannot use values as its oracle

pgcolumnar.analyze() writes through pg_restore_attribute_stats, which takes
VARIADIC "any" and validates each argument's type at run time. A mistyped
argument is not an error.
The function emits a WARNING, sets that argument to
NULL, and returns cleanly having stored nothing.

pg_restore_attribute_stats also leaves kinds it was not given in place — correct,
and fatal to attribution: a statistic we failed to write is still in the catalog
wearing core's shape, so every value assertion reads core's work and reports it as
ours. Core ANALYZE is therefore the oracle and the comparison is over SHAPE.

The refusal carries the shell suite's name (#1131)

On 15, 16 and 17 the shell suite declines through check_skip, which RECORDS the
refusal as "the differential analyze path". Until expect.cannot_run could carry a
name, no port could emit that string and the pair reported missing: 1 however
faithful the rest was. The precondition — the server major — exists on both sides,
which is the case resolution 1 unblocks.

One arm stronger than the original

analyze_differential.sh names "could not read the server major, so the gate below
cannot be trusted" inside a pgc_fail that fires only when the version is
unreadable. The port asserts the same property on every run, under the same
name, so the gate below it is known to have been decided on a real number rather
than on an empty string that compared less-than 18. That is why the port runs 27
checks on 18 where the shell suite runs 26.

Independent at every seam

shell port
pairing kind/operator/collation an ad_shape table, unnest ... WITH ORDINALITY joined three ways five slots read as three arrays, zipped in Python
the WARNING grep -c WARNING over psql's output a psycopg notice handler
frequencies one set-returning query counting disagreements one parameterised count per most-common value, so a failure names the value

The shell materialises a table because every q() opens its own connection. The
port holds one, so it does not need to.

The element type is deliberately not compared, on both sides.
pg_statistic.stavalues1 is declared anyarray, so pg_typeof returns the
constant string anyarray for every row ever stored — a probe built on it reports
every slot as mismatched, core's own included.

Removal proof

Three mutations of pgcolumnar--1.0-alpha4.sql. Each asserted that its anchor
matched exactly once and that the file changed, and each was restored and
re-hashed byte-identical before the next ran. Control: 27 checks passed.

cell mutation reddens
A 27 pass
B mcvfreqs::real[]::float8[] and without a WARNING, which is how pg_restore_attribute_stats drops an argument, got 10 want 0
C frequency divided by the non-null count every most-common value of i exists with exactly its stored frequency, got 2 want 0
D 'histogram_bounds', hist::text → a typed NULL and a histogram for exactly the four core gave one, got 0 want 4

Cell B is the failure mode the suite is named for, and the notices it produced say
why a value oracle cannot see it:

argument "most_common_freqs" has type double precision[], expected type real[]
argument "most_common_freqs" must be specified when argument "most_common_vals" is specified

Ten of them — five columns, twice each, because the pair rule drops
most_common_vals along with the mistyped most_common_freqs. The call still
succeeded.

The five-major gate

PG15  2422 pass + 0 fail + 2 unrun   rc=67  EXIT_INCOMPLETE
PG16  2422 pass + 0 fail + 2 unrun   rc=67  EXIT_INCOMPLETE
PG17  2422 pass + 0 fail + 2 unrun   rc=67  EXIT_INCOMPLETE
PG18  2454 pass + 0 fail + 0 unrun   rc=0   ALL GREEN
PG19  2457 pass + 0 fail + 0 unrun   rc=0   ALL GREEN

The 67 is not new and the classification does not move. test_temporal already
declined on 15, 16 and 17 for the same reason before this branch; this port joins it
in the same three legs. The CI cluster job runs PG18 only, where the corpus is green.

cluster_tests 441 → 442 and guard_tests 382, both derived by collection on
the tree as rebased onto c050f84, never by adding a delta.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

20 names, `missing: 0`, and no extras.

`pgcolumnar.analyze()` writes through `pg_restore_attribute_stats`, which takes
`VARIADIC "any"` and validates each argument's type at run time. A mistyped
argument is not an error: the function warns, sets the argument to NULL and
returns cleanly having stored nothing. A value-level suite cannot see that,
because the function leaves kinds it was not given in place -- so a statistic we
failed to write is still in the catalog wearing core's shape, and every value
assertion reads core's work and reports it as ours. Core ANALYZE is therefore the
oracle and the comparison is over SHAPE.

The refusal carries the shell suite's name, which commandprompt#1131 made possible: on 15, 16
and 17 `analyze_differential.sh` declines through `check_skip`, which RECORDS the
refusal as "the differential analyze path". The port declines under the same name
and the run exits 67, joining `test_temporal` on those majors.

ONE ARM STRONGER THAN THE ORIGINAL. The shell suite names "could not read the
server major, so the gate below cannot be trusted" inside a `pgc_fail` that fires
only when the version is unreadable. The port asserts it on every run, so the
version gate below it is known to have been decided on a real number rather than
on an empty string that compared less-than 18.

Independent at every seam: the shell materialises an `ad_shape` table and pairs
kind/operator/collation with `unnest ... WITH ORDINALITY` joined three ways, while
this reads the five slots as three arrays and zips them in Python; the shell greps
psql's output for WARNING while this registers a psycopg notice handler;
frequencies are checked one value at a time through a parameterised count, so a
failure names the value that disagreed.

Proved by removal against `pgcolumnar--1.0-alpha4.sql`, each mutation asserted to
have matched its anchor exactly once and to have been restored byte-identical;
control 27 checks passed:

    mcvfreqs::real[] -> ::float8[]   and without a WARNING ...          got 10 want 0
    frequency / non-null count       every most-common value of i ...   got 2 want 0
    histogram_bounds -> typed NULL   and a histogram for exactly ...     got 0 want 4

`cluster_tests` 441 -> 442, derived by collection. `guard_tests` re-derived in the
same run and did NOT move: 382.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs
@jdatcmd

jdatcmd commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Verified rather than read, on both sides of the version gate, and the claim I went looking to break is the one that held hardest.

The #1131 property, confirmed at RUNTIME and not by the static grader

compare_to_bash reads source, so missing: 0 proves the string is present, not that anything emits it. Those are different claims, and this is the first pair where the difference is the whole point. So I spied on Expect._record and ran it on PG 17:

RECORD  name='could not read the server major, so the gate below cannot be trusted'  verdict=PASS
RECORD  name='the differential analyze path'                                         verdict=UNRUN

Against the shell suite's own record on the same server:

RESULT  analyze_differential  analyze_differential  the differential analyze path  SKIP  17

Same string, and UNRUN rather than PASS. The name arrives without the declaration being laundered into a pass, which is the trap the iceberg_fdw INCOMPLETE entry refused and the one a "make the grader happy" fix would have fallen into.

Worth noting because the UNRUN display line shows the reason code and detail, not the name — so reading the terminal output alone would not have told me this worked. The record is where the name lives.

"One arm stronger" — measured on both majors

PG 18   shell 26 passed + 0 failed        port 27 pass + 0 fail + 0 unrun
PG 17   shell  1 skipped                  port  1 pass + 0 fail + 1 unrun

The +1 is the same arm on both majors, and the spy output above shows exactly what it is: the version-read premise recorded as a PASS even on the major where everything below it declines. The original only fires that name inside a pgc_fail when the version is unreadable, so on a healthy run it asserts nothing. Yours asserts it always, which is what makes "major < 18" a decision about a real number rather than about an empty string comparing less-than 18.

The oracle argument, which is the part I would have got wrong

A mistyped argument is not an error. The function emits a WARNING, sets that argument to NULL, and returns cleanly having stored nothing.

Combined with pg_restore_attribute_stats leaving untouched kinds in place, that is a precise and genuinely nasty trap: a statistic we failed to write is still in the catalog wearing core's shape, so every value assertion reads core's work and reports it as ours. Comparing SHAPE against core ANALYZE as the oracle is the right response, and it is the kind of thing that only shows up if you go looking for how the arm could pass while doing nothing.

Independence

The three seams differ in ways that fail differently, which is the test worth applying:

pairing        a materialised table + unnest WITH ORDINALITY   vs  three arrays zipped in Python
the WARNING    grep -c over psql output                        vs  a psycopg notice handler
frequencies    one set-returning query counting disagreements  vs  one parameterised count per MCV

The last one is not just independence, it is a better failure: the port names the offending value instead of reporting a count. And your reason for the difference is the honest one — the shell materialises because every q() opens its own connection, not because a table is better.

Parity, re-derived here

literal matches: 18 | template matches: 2 | missing: 0
VERDICT: every bash property is covered
ASSERTIONS IN THE PORT AND NOT IN THE BASH SUITE: none

Not a finding, recorded so nobody chases it

My first run of analyze_differential.sh in a fresh clone died in the objstore submake (No rule to make target 'all'). It reproduces identically on a main-based clone and disappears when the suite is given an explicit pg_config, so it is my invocation racing my own concurrent build across two majors in one tree — not your PR, and not a defect I am reporting. Saying so because a build failure in a review is the sort of thing that gets half-remembered as a real one.

Approving once the two suites legs land; everything else is green and the review is finished. CI at 91eb3385: 12 success, 0 failure, 2 pending.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 91eb3385. 14 of 14 on this sha, non-green 0, CLEAN — including both suites legs.

The verification is in my comment above and stands unchanged: the #1131 name is emitted at RUNTIME as name='the differential analyze path' with verdict UNRUN, matching the shell's own record on PG 17; the extra arm is measured on both majors (shell 26 / port 27 on 18, shell 1 / port 2 on 17); parity re-derived here at 18 literal + 2 template, missing 0, no extras.

The oracle argument is the part that earns this: a mistyped argument to pg_restore_attribute_stats is a WARNING and a NULL, and untouched kinds stay in the catalog wearing core's shape — so a value assertion reads core's work and reports it as ours. Comparing shape against core ANALYZE is the right answer to a trap that only appears if you ask how the arm could pass while doing nothing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

@jdatcmd
jdatcmd merged commit e7eafcf into commandprompt:main Sep 19, 2026
14 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.

2 participants