Skip to content

test/pytest: port the differential type matrix, and three arms it cannot keep (#432) - #1020

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:port/432-differential-type-matrix
Sep 12, 2026
Merged

test/pytest: port the differential type matrix, and three arms it cannot keep (#432)#1020
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:port/432-differential-type-matrix

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

First slice of the #432 port, and the interesting part is what it would not reproduce.

test/differential.sh is the largest suite in the tree and its governing property is the
right one: load the same data into a heap table and a columnar one, and every query must
answer identically. Heap is the oracle, so it catches encode/decode, null-handling and
chunk-skipping bugs generically.

This ports part 1, the type matrix: twenty columns, 12,000 rows, a different null modulus
per column so no two share a pattern, small chunk-group and stripe limits so there is
something to skip. 60 tests, 129 counted checks, 3.5 seconds. Names are the bash suite's
character for character, so compare_to_bash.py can diff the two by property.

Three of the bash arms cannot fail

Each was found by the vacuity layer refusing a comparison, then measured on a cluster.

1. c_int eq probes a value that is never present. c_int is g*7-100, so c_int = 600
needs g=100, and 100%5=0 puts a NULL there.

rows with c_int = 600:       0
which g would give 600:      100
is that row NULL (g%5=0)?    t
pgc_set_hash of that query:  EMPTY      <- both sides

Both sides hash to EMPTY, compare equal, and the arm passes having compared nothing to
nothing. The port keeps it with a stated reason and adds c_int eq present on 607, which
g=101 supplies and no null modulus touches.

2. c_ztext is null asks for nulls in a column that has none. c_ztext is
CASE WHEN g%2=0 THEN '' ELSE 'z'||g END.

rows with c_ztext IS NULL:   0
rows with c_ztext = '':      6000

Same vacuity. The port asserts the empty-string count instead, which a decoder confusing
'' with NULL would move -- and that is the property the suite tests explicitly elsewhere
under "empty-vs-null".

3. c_f4 sum/avg and c_f8 sum/avg assert exact equality of a float sum. A float sum has
no single right answer; it has one per summation order. Measured on heap alone, one table,
three row orders, extra_float_digits = 3:

ORDER BY id        -0.27597385772197924
ORDER BY id DESC   -0.2759738577219848
ORDER BY c_f8      -0.2759738578545523

Three answers from one access method, so "columnar equals heap exactly" is false by
construction. The bash arm passes because pgc_set_hash hashes the text rendering and
psql's default precision rounds the difference away at some magnitudes and not others -- a
real tolerance, implicit and magnitude-dependent. My own first probe of this was fooled by
exactly that: both sides printed 3.377356e+07 while the values were 33773560 and 33773584.

The port states the tolerance: 1e-6 relative for the two float columns, exact for int,
bigint, smallint and numeric where a tolerance would hide the defect the arm exists to find,
and a control that the bound is tight enough to have a direction.

I am not proposing the bash suite be changed in this PR. The three arms are named here and in
TESTS.md so the next person does not port them as-is; whether to fix them in place is yours.

Choices worth the reviewer's time

Row lists, not hashes. lib.sh hashes because bash has no structured result. Comparing
rows prints what differs instead of two hex strings, and lets the vacuity layer see the
both-sides-empty case -- which is how all three findings above surfaced. A hash cannot.

Module-scoped fixture. 12,000 rows over twenty columns is too slow to rebuild for each of
sixty tests. That costs pgc_conn's write watch, so the load asserts its own row count,
chunk-group count and stripe count. Those three premise arms are what watch_writes would
otherwise have done, and they are not decoration.

My first version of them queried pgcolumnar.chunk_group, which does not exist -- the server
said so. A guessed catalog name is a premise arm that errors instead of asserting; the real
ones are zone_map and row_group via get_storage_id, which is what lib.sh's own
chunk_group_count and stripe_count read.

Test plan

Where this leaves #432

Part 1 of one suite. The suite has seven parts and the tree has 248 product suites at
0.66% coverage (36 of 5,453 distinct bash check names, measured against a five-major
matrix). The remaining parts of differential are boundary conditions, lightweight
encodings, aggregates with deletes, bloom equality skipping, wide projections and the
covering count(*) -- each a natural next slice on the same fixture pattern.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@linuxhikerpm linuxhikerpm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The three bash arms that cannot fail are the part worth keeping: c_int = 600 lands on a NULL, c_ztext IS NULL is empty by construction, and a float sum has no single exact answer. Heap as oracle, row lists rather than hashes, stripe_row_limit => 5000, and a module fixture that asserts its own row/chunk/stripe counts are the right seams.

Two holds before an approval:

  1. Rebase onto current main. #1012 already took TESTS.md section 29 for test_join_vector_agg.py. This PR adds another section 29.
  2. It depends on #1018 for the job that would actually run this file. Until that lands, a green pytest-guards job here does not exercise these 60 tests.

Do not merge.

…not keep (commandprompt#432)

The governing property of test/differential.sh: load the same data into a heap and
a columnar table and every query must answer identically. This ports part 1, the
type matrix -- twenty columns, 12,000 rows, a different null modulus per column,
across many chunk groups and several stripes.

60 tests, 129 counted checks, 3.5 seconds. Names are the bash suite's character
for character, so compare_to_bash.py can diff the two by property.

THREE OF THE BASH ARMS CANNOT FAIL, and the port says so rather than reproducing
them. Each was found by the vacuity layer refusing a comparison, then measured.

1. `c_int eq` probes `c_int = 600`. c_int is g*7-100, so 600 needs g=100, and
   100%5=0 puts a NULL there. Measured: 0 rows. Both sides hash to EMPTY, compare
   equal, and the arm passes having compared nothing to nothing. The port keeps it
   with a stated reason and adds `c_int eq present` on 607, which g=101 supplies
   and no null modulus touches.

2. `c_ztext is null` asks for nulls in a column that has none:
   CASE WHEN g%2=0 THEN '' ELSE 'z'||g END is never NULL. Measured: 0 rows IS NULL,
   6000 rows = ''. Same vacuity. The port asserts what can fail instead -- the
   empty-string count -- because a decoder that confused '' with NULL would move it.

3. `c_f4 sum/avg` and `c_f8 sum/avg` assert exact equality of a float sum, which
   has no single right answer. MEASURED on heap alone, one table, three row orders,
   extra_float_digits=3:

       ORDER BY id        -0.27597385772197924
       ORDER BY id DESC   -0.2759738577219848
       ORDER BY c_f8      -0.2759738578545523

   Three answers from one access method, so "columnar equals heap exactly" is false
   by construction. The bash arm passes because pgc_set_hash hashes the TEXT
   rendering and psql's default precision rounds the difference away at some
   magnitudes and not others -- a real tolerance, implicit and magnitude-dependent.
   The port states it: 1e-6 relative for the two float columns, exact for int,
   bigint, smallint and numeric, where a tolerance would hide the defect the arm
   exists to find. With a control that the bound is tight enough to have a
   direction.

Row lists rather than hashes: a failure prints the rows that differ instead of two
hex strings, and the vacuity layer can see a both-sides-empty comparison, which a
hash cannot.

The fixture is module-scoped because 12,000 rows over twenty columns is too slow to
rebuild per assertion. That costs pgc_conn's write watch, so the load asserts its
own row count, chunk-group count and stripe count -- those three premise arms are
what watch_writes would otherwise have done.

My first version of those premises queried pgcolumnar.chunk_group, which does not
exist. A guessed catalog name is a premise arm that errors instead of asserting;
the real ones are zone_map and row_group via get_storage_id, which is what lib.sh's
own helpers read.

Depends on commandprompt#1018 for the CI job that runs it: this file needs a cluster, so until
that lands it is in the half nothing runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the port/432-differential-type-matrix branch from 1c8e3a4 to b1416dd Compare September 12, 2026 19:04

@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. The port is good and the three arms it would not reproduce are better. I verified the first one arithmetically rather than taking it.

c_int eq compares nothing to nothing, confirmed

c_int  = CASE WHEN g%5=0 THEN NULL ELSE (g*7-100) END      differential.sh:57
arm    = SELECT id FROM %T WHERE c_int = 600               differential.sh:121

g*7-100 = 600  ->  g = 100  ->  100 % 5 == 0  ->  NULL

Zero rows on both sides, both hash to EMPTY, the arm passes having compared nothing. Adding c_int eq present on 607 — which g=101 supplies and no null modulus touches — is the right repair, and keeping the original with a stated reason is better than deleting it.

The float finding is the one I would put in front of anyone writing a differential suite

c_f8 = sqrt(g)*(±1), summed over 12,000 rows by diff_query "$c sum/avg" at differential.sh:104, which compares exactly. A float sum has one answer per summation order, and you measured three from heap alone:

ORDER BY id        -0.27597385772197924
ORDER BY id DESC   -0.2759738577219848
ORDER BY c_f8      -0.2759738578545523

So "columnar equals heap exactly" is false by construction, and the bash arm passes only because pgc_set_hash hashes the text rendering — an implicit, magnitude-dependent tolerance nobody chose. That your own first probe was fooled by 3.377356e+07 standing for both 33773560 and 33773584 is the detail that makes it credible.

Stating the tolerance — 1e-6 relative for the two float columns, exact for int/bigint/smallint/numeric where a tolerance would hide the defect — plus a control that the bound has a direction, is exactly the right shape.

Row lists over hashes

A hash cannot [see the both-sides-empty case].

That is why all three findings surfaced at all, and it is the concrete answer to jd's "we already know the pytest tests are of higher accuracy": not a general claim about the harness, but three arms in the largest suite in the tree that cannot fail, found by porting them.

Verified

  • Independence holds: the lib.sh references are prose and comments, which the rule allows explicitly. No import, no subprocess of the shell harness.
  • Your rebase took section 30, which is the same resolution I reached independently before your push landed first. My open #1022 also claimed 30, so I am renumbering mine to 31 rather than asking you to move.

Not proposing the bash suite change here either — the three arms are named in TESTS.md so the next porter does not carry them across as-is, and whether to repair them in place is jd's.

jdatcmd added a commit that referenced this pull request Sep 12, 2026
#1020 rebased onto main and took section 30 for test_differential.py. This branch
claimed the same number. Heading, contents entry and anchor moved together.

FOURTH TIME TODAY on this one anchor: #1007 and #1012 both claimed 28, #1012 and
#1020 both claimed 29, and now this and #1020 both claimed 30. Every change appends
a section with the next sequential number, so any two open at once collide by
construction. That is #996's shape in TESTS.md rather than in the CHANGELOG, and it
is worth saying that renumbering by hand each time is the cost of not fixing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
@jdatcmd
jdatcmd merged commit 9663f9b into commandprompt:main Sep 12, 2026
13 checks passed
jdatcmd added a commit that referenced this pull request Sep 12, 2026
#1020 landed. Two additive collisions, both resolved by keeping both sides:
CHANGELOG entries, and TESTS.md sections. Section 31 survives -- #1020 took 30 as
expected, so the renumber one commit back was correct and no further move is needed.

    section numbers 29, 30, 31, no duplicates, every anchor resolves
    the 14 database-free files   0 fail

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
OffgridwithJD added a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 12, 2026
All seven parts of test/differential.sh now have a pytest twin. 85 tests, 232
checks, 17 seconds. Parts 1 and 2 were commandprompt#1020 and the first commits here; this adds
boundary conditions, lightweight encodings, aggregates with deletes, bloom equality
skipping, wide projections and the covering count.

A FOURTH UNFALSIFIABLE BASH ARM, and the subtlest of the four. `textbloom
collate-mismatch` probes `tk = 'k100'`, and tk is 'k' || ((g*2654435761)%50000)
over 16,000 rows of a 50,000-wide domain -- 32% coverage, and measured, k100
matches 0 rows. The arm exists to catch a bloom wrongly pushed under a mismatched
collation, and a wrongly-pushed bloom skips the chunks holding the match and
returns 0. So the one defect it is built to detect produces exactly the answer it
expects. Found by a premise arm I added asking whether the probe value exists.

The port probes a value derived from the data, asserts it is present exactly once,
and adds the direction the arm must fail in -- that the mismatched collation
RETURNS the row rather than skipping it.

FIXTURE PREMISES ARE ASSERTED WHERE THEY DECIDE WHAT IS UNDER TEST:

  * the bloom fixture's key spread, at least 90,000 of the domain per chunk.
    Ordered keys would make every bloom arm pass on zone maps alone and say nothing
    about blooms, and no arm would report that.
  * the dictionary fixture's cardinalities, 4 and 6 against an md5 per row, so the
    per-column verdict is visible rather than assumed.
  * the delete in part 4 removed exactly 400 rows. A DELETE matching nothing leaves
    the per-group fallback untested while every arm stays green.
  * the update in part 7 leaves a count neither the delete nor the update alone
    would give, which is the arithmetic the metadata path has to get right.

Float aggregates use min/max rather than sum, because a float sum has no single
right answer -- part 1 measures three from heap alone by row order.

SET on the connection rather than ALTER DATABASE in part 7: that form exists
because each psql invocation in the bash suite is a new session, and this is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
OffgridwithJD added a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 12, 2026
All seven parts of test/differential.sh now have a pytest twin. 85 tests, 232
checks, 17 seconds. Parts 1 and 2 were commandprompt#1020 and the first commits here; this adds
boundary conditions, lightweight encodings, aggregates with deletes, bloom equality
skipping, wide projections and the covering count.

A FOURTH UNFALSIFIABLE BASH ARM, and the subtlest of the four. `textbloom
collate-mismatch` probes `tk = 'k100'`, and tk is 'k' || ((g*2654435761)%50000)
over 16,000 rows of a 50,000-wide domain -- 32% coverage, and measured, k100
matches 0 rows. The arm exists to catch a bloom wrongly pushed under a mismatched
collation, and a wrongly-pushed bloom skips the chunks holding the match and
returns 0. So the one defect it is built to detect produces exactly the answer it
expects. Found by a premise arm I added asking whether the probe value exists.

The port probes a value derived from the data, asserts it is present exactly once,
and adds the direction the arm must fail in -- that the mismatched collation
RETURNS the row rather than skipping it.

FIXTURE PREMISES ARE ASSERTED WHERE THEY DECIDE WHAT IS UNDER TEST:

  * the bloom fixture's key spread, at least 90,000 of the domain per chunk.
    Ordered keys would make every bloom arm pass on zone maps alone and say nothing
    about blooms, and no arm would report that.
  * the dictionary fixture's cardinalities, 4 and 6 against an md5 per row, so the
    per-column verdict is visible rather than assumed.
  * the delete in part 4 removed exactly 400 rows. A DELETE matching nothing leaves
    the per-group fallback untested while every arm stays green.
  * the update in part 7 leaves a count neither the delete nor the update alone
    would give, which is the arithmetic the metadata path has to get right.

Float aggregates use min/max rather than sum, because a float sum has no single
right answer -- part 1 measures three from heap alone by row order.

SET on the connection rather than ALTER DATABASE in part 7: that form exists
because each psql invocation in the bash suite is a new session, and this is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
OffgridwithJD added a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 12, 2026
All seven parts of test/differential.sh now have a pytest twin. 85 tests, 232
checks, 17 seconds. Parts 1 and 2 were commandprompt#1020 and the first commits here; this adds
boundary conditions, lightweight encodings, aggregates with deletes, bloom equality
skipping, wide projections and the covering count.

A FOURTH UNFALSIFIABLE BASH ARM, and the subtlest of the four. `textbloom
collate-mismatch` probes `tk = 'k100'`, and tk is 'k' || ((g*2654435761)%50000)
over 16,000 rows of a 50,000-wide domain -- 32% coverage, and measured, k100
matches 0 rows. The arm exists to catch a bloom wrongly pushed under a mismatched
collation, and a wrongly-pushed bloom skips the chunks holding the match and
returns 0. So the one defect it is built to detect produces exactly the answer it
expects. Found by a premise arm I added asking whether the probe value exists.

The port probes a value derived from the data, asserts it is present exactly once,
and adds the direction the arm must fail in -- that the mismatched collation
RETURNS the row rather than skipping it.

FIXTURE PREMISES ARE ASSERTED WHERE THEY DECIDE WHAT IS UNDER TEST:

  * the bloom fixture's key spread, at least 90,000 of the domain per chunk.
    Ordered keys would make every bloom arm pass on zone maps alone and say nothing
    about blooms, and no arm would report that.
  * the dictionary fixture's cardinalities, 4 and 6 against an md5 per row, so the
    per-column verdict is visible rather than assumed.
  * the delete in part 4 removed exactly 400 rows. A DELETE matching nothing leaves
    the per-group fallback untested while every arm stays green.
  * the update in part 7 leaves a count neither the delete nor the update alone
    would give, which is the arithmetic the metadata path has to get right.

Float aggregates use min/max rather than sum, because a float sum has no single
right answer -- part 1 measures three from heap alone by row order.

SET on the connection rather than ALTER DATABASE in part 7: that form exists
because each psql invocation in the bash suite is a new session, and this is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
OffgridwithJD added a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 12, 2026
All seven parts of test/differential.sh now have a pytest twin. 85 tests, 232
checks, 17 seconds. Parts 1 and 2 were commandprompt#1020 and the first commits here; this adds
boundary conditions, lightweight encodings, aggregates with deletes, bloom equality
skipping, wide projections and the covering count.

A FOURTH UNFALSIFIABLE BASH ARM, and the subtlest of the four. `textbloom
collate-mismatch` probes `tk = 'k100'`, and tk is 'k' || ((g*2654435761)%50000)
over 16,000 rows of a 50,000-wide domain -- 32% coverage, and measured, k100
matches 0 rows. The arm exists to catch a bloom wrongly pushed under a mismatched
collation, and a wrongly-pushed bloom skips the chunks holding the match and
returns 0. So the one defect it is built to detect produces exactly the answer it
expects. Found by a premise arm I added asking whether the probe value exists.

The port probes a value derived from the data, asserts it is present exactly once,
and adds the direction the arm must fail in -- that the mismatched collation
RETURNS the row rather than skipping it.

FIXTURE PREMISES ARE ASSERTED WHERE THEY DECIDE WHAT IS UNDER TEST:

  * the bloom fixture's key spread, at least 90,000 of the domain per chunk.
    Ordered keys would make every bloom arm pass on zone maps alone and say nothing
    about blooms, and no arm would report that.
  * the dictionary fixture's cardinalities, 4 and 6 against an md5 per row, so the
    per-column verdict is visible rather than assumed.
  * the delete in part 4 removed exactly 400 rows. A DELETE matching nothing leaves
    the per-group fallback untested while every arm stays green.
  * the update in part 7 leaves a count neither the delete nor the update alone
    would give, which is the arithmetic the metadata path has to get right.

Float aggregates use min/max rather than sum, because a float sum has no single
right answer -- part 1 measures three from heap alone by row order.

SET on the connection rather than ALTER DATABASE in part 7: that form exists
because each psql invocation in the bash suite is a new session, and this is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
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.

3 participants