Skip to content

test: set_options refuses an unknown compression name and keeps the stored one; encode_effort seeds the ledger (#1341) - #1371

Merged
jdatcmd merged 32 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-set-options-compression
Oct 15, 2026
Merged

jdatcmd merged 32 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-set-options-compression

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Part of #1341: the medium sql-objects item "set_options refuses an unknown compression name". The guard in pgcolumnar--1.0-alpha5.sql had no arm. Without it, an unknown name is stored, and the C reader maps a name it doesn't know to −1 and falls back to the instance default. So the table would compress as something else, with nothing said.

Arms (encode_effort.sh, beside the encode_effort refusal they mirror)

  • The refusal: set_options(compression => 'brotli') must raise unknown columnar compression "brotli".
  • The stored value: the zstd set just before it must still be the stored value.

PG17, main ff09c2c against this branch, with the compression-name check removed:

main this branch
encode_effort.sh 10/10 both red, "the compression set before it is still the one stored: got [brotli] want [zstd]"

Shell only: encode_effort has no pytest twin.

Evidence

🤖 Generated with Claude Code

@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 ed40487. Reproduced on PG18.

  • Green: 12/12.
  • Red: I replaced the compression-name guard in set_options (pgcolumnar--1.0-alpha5.sql:531-534) with a comment. Both arms fail:
    FAIL an unknown compression name is refused: got [no ( set_options …)] want [yes]
    FAIL and the compression set before it is still the one stored: got [brotli] want [zstd]
  • Ledger: the 12 seeded rows match the 12 checks, and the two dated rows are those two arms. No duplicate keys.

Before merge: it conflicts with main 9b3c181. Main's ceiling is already 245 from #1362, so the rebase must restate it to 244 and restate the census on top of 1821.

@OffgridwithJD

OffgridwithJD commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

Rebased onto 57f6e0e (#1401), on top of #1403. The new head is 65d4ff2. Only the count files changed, so this needs your re-approval.

  • Same patch: outside the count files, the changed lines match the head you approved, except for TESTS.md section renumbers where main's new sections took the old numbers.
  • The ledger is restated on main's census and agrees with its tsv: 2798 rows, 2105 never, 693 dated, read back from the file. Each changed paragraph's arithmetic starts from main's last numbers.
  • cluster_tests 566, by collection over the cluster files on this tree. guard_tests 433 is unchanged.
  • Runs on the rebased tree at 217f23b, PG17 and PG18: PG17 encode_effort: 12 passed + 0 failed + 0 unrunnable + 0 skipped; PG17 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped; PG18 encode_effort: 12 passed + 0 failed + 0 unrunnable + 0 skipped; PG18 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped. harness_selftest 1245/1245 and docs_style passed on both. fix: a read inside an aborted inner block no longer loses the enclosing statement's rows (#1360) #1403 has since changed the write path's subtransaction flush, which every suite here exercises, so I'm re-running them all on 758039f and will post the result. CI runs PG17 and PG18 on this head.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Re-run finished on the tree after #1403 (758039f or later). Everything is green, so the head above stands:

  • PG17 docs_style: passed
  • PG17 encode_effort: 12 passed + 0 failed + 0 unrunnable + 0 skipped
  • PG17 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped
  • PG18 docs_style: passed
  • PG18 encode_effort: 12 passed + 0 failed + 0 unrunnable + 0 skipped
  • PG18 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-set-options-compression branch from 65d4ff2 to cad2dfa Compare October 15, 2026 17:06
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 7da848d (#1432, #1409). The new head is cad2dfa; only the count files changed, so please re-approve this head.

🤖 Generated with Claude Code

@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.

Re-approved at cad2dfa. Compared with ed40487, the head I approved, file by file: the changed lines are the same, apart from the count files (check_ledger_budget.txt, expected_tests.txt) and, in #1378/#1379/#1380/#1383, the TESTS.md section number, now 115.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-set-options-compression branch from cad2dfa to 0a50162 Compare October 15, 2026 17:53
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 1f7b5da (#1411, #1412, #1417, #1418). The new head is 0a50162. Only the count files changed, plus TESTS.md section numbers where main took 115, so please re-approve this head.

🤖 Generated with Claude Code

@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.

Re-approved at 0a50162, the rebase onto 1f7b5da. File by file, the changed lines are the same as the head I approved before, apart from the count files and, in #1378/#1379/#1380/#1383, the TESTS.md section number.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

The PG18 re-run on this head (0a50162, on 1f7b5da with #1412's index-build change) is all green:

  • docs_style: passed
  • encode_effort: 12 passed + 0 failed + 0 unrunnable + 0 skipped
  • harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 37ea557 (#1414, #1425). The new head is 44c60ad. Only the count files changed, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-set-options-compression branch 2 times, most recently from 44c60ad to f65e22d Compare October 15, 2026 18:39
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto f2bd5f1 (#1427). The new head is f65e22d. Only the count files changed, so please re-approve this head.

🤖 Generated with Claude Code

@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.

Re-approved at f65e22d, the rebase onto f2bd5f1. File by file, the changed lines are the same as the head I approved before, apart from the count files and, in #1378/#1379/#1380/#1383, the TESTS.md section number.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto d5c0a5e (#1391). The new head is 714a628, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-set-options-compression branch from f65e22d to 714a628 Compare October 15, 2026 19:27
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 2d1a77f (#1413–#1424). The new head is c2dd031, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-set-options-compression branch from 714a628 to c2dd031 Compare October 15, 2026 20:58

@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.

Re-approved at c2dd031, the rebase onto 2d1a77f. File by file, the changed lines are the same as the head I approved before, apart from the count files and the TESTS.md section number.

🤖 Generated with Claude Code

jdatcmd and others added 7 commits October 15, 2026 21:15
…ws are reworded

The mutation column is a ';'-separated set, written back sorted. A --mutation
whose text held "; " was stored as two mutations, one a fragment starting with
a space, and every later merge re-sorted it. commandprompt#1427's recovery_minimal_wal text
did this to 8 rows, and commandprompt#1423, commandprompt#1424 and commandprompt#1426 each carried them as a change of
their own. Four older harness_selftest rows were split the same way.

- merge refuses a --mutation holding ';' (rc 2, ledger untouched).
- the reader refuses a ';'-piece with whitespace at an end, by line: the writer
  joins with a bare ';', so such a piece is a fragment, never a mutation.
- the 12 rows are reworded without ';', from their original text in history.
  Only field 6 changes; the census stays 3007 / 2282 / 725.

Red first: the two refusal arms got 0 want 2 on the unchanged tool, and the
committed-ledger arm was red at line 1783 with the refusal in and the rows not
yet reworded. guard_tests 435 -> 438 by collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment-mark mutation (commandprompt#1341 item 75)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…wn privileges, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…d the refused tier writes no file, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
… back; both its branches echoed "ran" (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
jdatcmd and others added 14 commits October 15, 2026 21:16
…x, and the twin fails it by name rather than raising (commandprompt#1383 review)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…name (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… arm runs with the configured list empty, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n comparing two empty strings (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d the shell names 'empty' itself (commandprompt#1379 review)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…l's name (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…0) and leaves nothing declared, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts connections (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restated for the chain you asked for. The new head is b8cd124, so please re-approve this head.

🤖 Generated with Claude Code

@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.

Re-approved at b8cd124, its place in the chain on 17420be. This PR's own commits, measured from the link below it, change the same lines as the head I approved before, apart from the count files and the TESTS.md section number.

🤖 Generated with Claude Code

jdatcmd and others added 7 commits October 15, 2026 22:00
…emise: prefix (commandprompt#1341)

Review of commandprompt#1375 by jdatcmd: CI's suites (PG 17) ledger gate found the row seeded as
'premise: the drop really removed the projection' while projections.sh:410 and
test_projections.py:795 name the check without the prefix, so the gate saw one
check not in the ledger and one orphan row. The row is renamed and keeps its
other fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e writing any statistics, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…blings do (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… dated (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ys so (commandprompt#1341)

test_a_named_column_that_does_not_exist_is_refused calls pgcolumnar.analyze(),
which needs PostgreSQL 18, so it declines on 15, 16 and 17 like the other
analyze_function twins. It was missing from expected_unrunnable.txt, so CI's
PG17 cluster job collected and passed 578 and still exited 67 on the
unexpected decline. Measured on PG17: test_analyze_function.py exits 67
without these rows and 0 with them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tored one (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dated (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD
OffgridwithJD force-pushed the test/1341-set-options-compression branch from b8cd124 to f41f38b Compare October 15, 2026 22:02
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restacked onto #1375's fix (875429d, its projections row renamed). The new head is f41f38b, so please re-approve this head. This PR's own commits are unchanged, and nothing in its diff moved.

🤖 Generated with Claude Code

@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.

Re-approved at f41f38b. This PR's own commits are the same as the head I approved, rebased onto the fixed #1375.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd merged commit 6422610 into commandprompt:main Oct 15, 2026
15 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