Repository navigation
Conversation
Reads and writes the Apache Thrift compact protocol without an IDL. Reader is a TProtocol-shaped cursor: readFieldBegin/readListBegin/ readMapBegin, scalar reads, skip(type), and readValue into a typed Value tree. Every read is bounds-checked; lengths and container sizes larger than the remaining input fail before allocating, nesting is limited by maxDepth, and decode returns a Result with a ThriftError. Binary values are views of the input. i64 reads as bigint, or as a checked number with readI64Number. Writer mirrors the reader. The fixtures hold 349 value trees with the bytes the official Python thrift 0.25.0 TCompactProtocol writes for them, plus seven structs cut from apache/parquet-testing files (footers, page headers, bloom filter header, offset and column index). The tests check that our bytes are identical, read every case back, and fuzz corrupted input.
🦋 Changeset detectedLatest commit: 81b9a19 The changes in this PR will be included in the next version bump. This PR includes changesets to release 35 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This was referenced Nov 15, 2026
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
Share one integer range check, decode strings with internal/utf8, drop the INVALID sentinel, the Reader's struct counter and the i64 bound constants, and build Writer container labels only on failure. Section banners go and the Reader and Writer docs no longer repeat their members.
Compare gold cases as Thrift.Value instead of converting back to JSON, use effect/encoding/Hex, and remove an identity helper and duplicated instructions from generate.py.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
Arrow chain (each PR is based on the one before it):
effect/encoding/FlatBuffers(base:main)effect/arrow(base:feat/encoding-flatbuffers)effect/dataframe(base:feat/arrow-support)Independent foundation, based on
mainand not part of the chain: this PReffect/encoding/Thrift, for the planned Parquet reader. A later Parquet to ArrowTable layer joins Parquet and Arrow.Arrow Flight, Protobuf / gRPC / ConnectRPC and Thrift's binary protocol / RPC envelopes are out of scope.
Why
A planned
effect/encoding/Parquetreads Parquet's footer (FileMetaData), page headers, column and offset indexes and bloom filter headers, all of which are Thrift compact-protocol structs. A later Parquet writer writes them. Footers of large files hold thousands of row groups × columns, so reading them is on the hot path of opening a file. Other Thrift users (Thrift RPC payloads, other formats built on Thrift) would need the same layer.Requirement
i64both as offsets/counts (safe integers) and as full 64-bit values.Alternatives
thriftnpm package: a runtime dependency; allocates an object per field header and readsi64asnode-int64; about 7× slower than this module's generic tree reader on the benchmark below.field_Nobjects withi64alwaysbigint; no writer, no limits, no sets/maps/uuid; about 4× slower than a typed reader on this module..thrift→ Effect Schema): possible later, on top of this layer.Typeuses Thrift's protocol-neutralTTypecodes so a binary protocol can share them.Summary
TProtocol: aboolfield's value lives in its header, soreadBool()right after aBoolheader reads no byte;readStructBegin/Endtrack the field-id deltas.Truncated) before allocating; varints are capped at 5/10 bytes; nesting is capped bymaxDepth(default 64, at most 1000 so the recursiveskipandreadValuestay within the stack).ThriftErrorkinds:Truncated,Malformed,InvalidUtf8,Range,Limit,Usage. The reader throws;decodereturns aResultand rethrowsUsage(caller bugs: invalid options, out-of-order calls, an invalidposition,skip/readValueof a type no input can produce), asFlatBuffers(Add effect/encoding/FlatBuffers #10643) does.initialSizea non-negative integer, elseUsage. Reader options areReaderOptions.i16values and field ids fail withMalformedinstead of wrapping (Python keeps them; Java/C++ wrap).Value, are views of the input: don't mutate the input while you use them.@since 4.1.5.Evidence
The oracle is the official Python library (
thrift0.25.0,TCompactProtocol), viatest/fixtures/thrift/generate.py, which reproducesgold.jsonbyte for byte (61 KB: 47 hand-written cases, one or more per compact-protocol rule, 20 seeded random structs and 7 structs cut from apache/parquet-testing).Truncated; mutation fuzzing of the corpus (in the test file) and a 600k-input one-off fuzz only ever producedThriftError.The original PR's benchmark (11.8 MB
FileMetaData, typed reader 144 ms vs hyparquet 584 ms and thethriftnpm package 1753 ms) has not been re-run during the takeover. A micro-benchmark on a 1.7 KB footer shows no change from the original implementation when reading or skipping it.readStringis about 3.5× faster on short ASCII strings since it moved to the sharedinternal/utf8decoder.Status
The takeover's assessment, fixes, focused tests, independent review and simplification pass are done.
Merge Danger
Door: two-way
A new experimental module; nothing existing changes except one line in the
effect/encodingbarrel.Blast Radius: none
No existing exports change. The fixture
gold.jsonadds 61 KB to the repo (excluded from dprint).Closes EFF-3816