Skip to content

chore: upgrade CRoaring to 4.7.1 - #1001

Merged
wgtmac merged 2 commits into
apache:mainfrom
manuzhang:upgrade-croaring-4.7.1
Oct 12, 2026
Merged

wgtmac merged 2 commits into
apache:mainfrom
manuzhang:upgrade-croaring-4.7.1

Conversation

@manuzhang

@manuzhang manuzhang commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Part of #1000. Include the upstream portable IO endian fix and check canonical little-endian bitmap bytes when serializing and reading.

Include the upstream portable IO endian fix and check canonical
little-endian bitmap bytes when serializing and reading.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 16:23

Copilot AI 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.

🟢 Approval recommended

The dependency update includes the upstream endian fix and the regression test validates both serialization and reading.

0 open findings

What changed in this PR

Upgrades bundled CRoaring to fix big-endian portable serialization identified in #1000.

Changes:

  • Updates CRoaring from 4.4.3 to 4.7.1.
  • Adds canonical little-endian serialization and deserialization coverage.
File Description
cmake_modules/​IcebergThirdpartyToolchain.cmake Updates the bundled CRoaring URL.
src/​iceberg/​test/​roaring_position_bitmap_test.cc Verifies canonical portable wire bytes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@manuzhang
manuzhang requested a review from wgtmac October 11, 2026 16:40
Preserve the fixture's embedded zeros and exact wire length while
satisfying modernize-avoid-c-arrays.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 17:52

Copilot AI 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.

🟢 Approval recommended

The dependency upgrade is valid and the regression test verifies the intended wire format.

0 open findings

🧠 Review effort: Balanced

@zhjwpku zhjwpku added the ready to merge This PR has been approved and it is ready to merge. label Oct 11, 2026
@wgtmac wgtmac removed the ready to merge This PR has been approved and it is ready to merge. label Oct 12, 2026
@wgtmac
wgtmac merged commit 274d1bb into apache:main Oct 12, 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.

4 participants