Skip to content

iceberg REST: the list cap's message says MiB, the unit it counts - #2051

Merged
jdatcmd merged 1 commit into
mainfrom
fix/rest-list-cap-mib
Nov 9, 2026
Merged

jdatcmd merged 1 commit into
mainfrom
fix/rest-list-cap-mib

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

A follow-up to #2047 (issue #2046), as promised in my review of #2050.

What changes

Test first

The paging test now asks for the phrase larger than 64 MiB across its pages.

Red, on main's code (PG18):

E   AssertionError: paging: 17 pages of 4 MiB (68 MiB in all) are refused with 54000, not held in memory: got '54000' want '54000 over the cap'
1 failed, 3 passed, 126 deselected

Green: test_iceberg_rest_scan.py passes on both majors.

PostgreSQL 18.6:   accounting: 130 pass + 0 fail + 0 unrun = 130   130 passed
PostgreSQL 17.11:  accounting: 130 pass + 0 fail + 0 unrun = 130   130 passed
docs_style.sh: PASSED

There are no registry changes, because no test was added or removed.

🤖 Generated with Claude Code

)

REST_MAX_RESPONSE is 64 * 1024 * 1024 bytes, which is 64 MiB. #2047's message
and CHANGELOG entry said "64 MB". The docs (#2050) and the reply cap's own
message give the size in MiB or bytes. The test now asks for "MiB"; on main
it got the bare 54000 because the message lacked the phrase.

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

jdatcmd commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD This is a small follow-up to #2047, which I mentioned in my #2050 review: the cap message now says MiB. It's test-first; the red and green runs are in the description.

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

@jdatcmd Approved at 965ff24.

  • The fix is right: REST_MAX_RESPONSE / (1024 * 1024) is mebibytes, so the message now names the unit it measures. The test's expected text and the CHANGELOG entry change with it.
  • Nothing else needs it: I searched src/ and objstore/ for other user-facing messages that divide by 1024×1024 and print "MB". There are none; the remaining hits are code comments. The docs already say MiB here (sql-reference.md, from #2050), so no doc change is needed.
  • Checks and merge: all 15 checks pass, and it merges cleanly with main at f2075fd.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd merged commit d19c5f8 into main Nov 9, 2026
15 checks passed
jdatcmd added a commit that referenced this pull request Nov 9, 2026
Brings in #2049, #2050 and #2051. TESTS.md conflicted on section 360: main's
test_docs_links_resolve.py keeps 360, and this branch's section becomes 361.
cluster_tests is 5365 by collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jdatcmd added a commit that referenced this pull request Nov 9, 2026
Brings in #2049, #2050 and #2051. TESTS.md conflicted on section 360: main's
test_docs_links_resolve.py keeps 360, and this branch's section becomes 361.
cluster_tests is 5372 by collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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