Skip to content

GH-47824: [Format] Clarify IPC footer offsets and metadata lengths - #54987

Open
kshitijprasaddev wants to merge 1 commit into
apache:mainfrom
kshitijprasaddev:docs/ipc-footer-metadata-length
Open

kshitijprasaddev wants to merge 1 commit into
apache:mainfrom
kshitijprasaddev:docs/ipc-footer-metadata-length

Conversation

@kshitijprasaddev

@kshitijprasaddev kshitijprasaddev commented Oct 22, 2026 •

Copy link
Copy Markdown

Rationale for this change

The encapsulated message's metadata_size excludes its framing prefix, whereas the file footer's Block.metaDataLength includes it. The current Block.offset comment also suggests an offset past the message header, while the writer records the start of the encapsulated message. This ambiguity makes implementing an IPC reader or writer harder.

Closes #47824.

What changes are included in this PR?

Clarify Block.offset and Block.metaDataLength in format/File.fbs, and explain the distinction in the columnar format documentation. Modern framing has metaDataLength = 8 + metadata_size; legacy framing without the continuation indicator uses 4 + metadata_size. The body starts at offset + metaDataLength.

Are these changes tested?

  • Checked the comments against PayloadFileWriter::WritePayload, WriteMessage, and the reader's block-offset handling in the current C++ sources.
  • Generated eight real IPC files with PyArrow 26.0.0: modern/legacy framing, primitive/dictionary arrays, and one/two record batches. Parsed the actual FlatBuffers footers with the Python FlatBuffers runtime and verified all 16 dictionary/record-batch blocks: prefix bytes at the stored offset, metadata-length equations, exact body start/bytes, and batch roundtrips.
  • sphinx-lint --enable all --disable dangling-hyphen,line-too-long docs/source/format/Columnar.rst passed with the repository-pinned sphinx-lint 0.9.1.
  • git diff --check passed. Compared File.fbs before/after with comment lines removed: schema declarations are unchanged.
  • No full Sphinx documentation build, C++ build, or upstream test-suite run is claimed.

Are there any user-facing changes?

Documentation and schema comments only; no schema layout, serialized format, or API changes.

Was AI used for this PR?

Codex investigated the issue, drafted the documentation and PR text, reviewed the diff against the implementation, and ran the local validation described above. Human review is pending.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #47824 has been automatically assigned in GitHub to PR creator.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #47824 has been automatically assigned in GitHub to PR creator.

1 similar comment
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #47824 has been automatically assigned in GitHub to PR creator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Format] Encapsulated message format metadata_size ambiguity.

1 participant