[FLINK-40370][python] Frame bytes-backed decimals as a bytes field - #28996
Open
vbhanuchander-lang wants to merge 1 commit into
Open
[FLINK-40370][python] Frame bytes-backed decimals as a bytes field#28996vbhanuchander-lang wants to merge 1 commit into
vbhanuchander-lang wants to merge 1 commit into
Conversation
FLINK-37192 replaced avro-python3 with avro>=1.12.0, which routes a bytes-backed decimal logical type through write_decimal_bytes and read_decimal_from_bytes. FlinkAvroEncoder and FlinkAvroDecoder override the primitive writes for JVM compatibility but not those two, so the payload was sized with write_long -- a fixed 8-byte long here -- where the JVM frames a bytes field with a 4-byte int. A JVM GenericDatumReader therefore consumed the size as the entire bytes field and every following field shifted: a record of amount=12.34, tail=7 encoded as 000000000000000204d200000007 and read back as an empty decimal with tail=2, leaving four bytes unread. It decoded without error, so this was silent corruption rather than a failure. Flink's own reader made the same mistake symmetrically, which is why it only shows up when a generic record crosses the JVM/Python boundary. Override both methods to frame the payload as a bytes field. The payload itself is unchanged -- the same two's-complement big-endian unscaled value avro already produced -- so only the length prefix moves. Verified against avro 1.12.2: the encoded record is now byte-identical to the JVM encoding, and the unscaled payload matches standard avro across positive, negative, zero and byte-boundary values.
Collaborator
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.
What is the purpose of the change
Bytes-backed Avro decimal logical types were encoded incompatibly with Flink's JVM serializer when a
GenericRecordcrossed the JVM/Python boundary, silently corrupting the record.FLINK-37192 replaced
avro-python3withavro>=1.12.0. That version routes a bytes-backed decimalthrough
BinaryEncoder.write_decimal_bytesandBinaryDecoder.read_decimal_from_bytes, both ofwhich size the payload with
write_long/read_long.FlinkAvroEncoderandFlinkAvroDecoderoverride the primitive reads and writes for JVM compatibility but not those two, and in this encoder
write_longis a fixed 8-byte long — so the length prefix was written as 8 bytes where the JVMframes a bytes field with a 4-byte int.
For a record of
amount=Decimal("12.34"), tail=7:A JVM
GenericDatumReaderconsumes the oversized prefix as the whole bytes field, so every followingfield shifts: the decimal reads back empty and
tailreads back as2, leaving four bytes unread.Nothing is raised, which is why this is silent corruption rather than a decode failure. Flink's own
reader is wrong in the same direction, so a pure-Python round trip looks correct and the defect only
appears across the boundary.
This does not affect standard Avro interoperability —
avro,fastavroand ordinary Kafka Avropayloads are unchanged. It is specific to Flink's internal JVM-compatible serializer.
Brief change log
FlinkAvroEncoder.write_decimal_bytesframes the unscaled payload as a bytes field rather thanletting avro size it with an 8-byte long
FlinkAvroDecoder.read_decimal_from_bytesreads that 4-byte size symmetricallyproduced, so only the length prefix moves
Verifying this change
This change added tests and can be verified as follows:
pyflink/fn_execution/tests/test_avro_format.py, four tests: the exact encoded bytes for adecimal followed by an int; that the record reads back field by field with JVM framing; that the
unscaled payload is unchanged for
0,±12.34and the±1.28/-1.29byte-boundary cases; anda round trip through
FlinkAvroDatumWriter/FlinkAvroDatumReaderavro.io.BinaryEncoder.write_decimal_bytesacross 23 values — positive, negative, zero and the±128, ±256, ±32768 boundaries — so only the framing differs
A note on how that was run: the module imports only
structandavro, and I exercised the testbody with
PyFlinkTestCasesubstituted byunittest.TestCasein a virtualenv with avro 1.12.2. Idid not build a full PyFlink environment locally, so the assertions are verified but the change has
not been through the project's own Python test harness on my machine. flake8 is clean under
flink-python/tox.ini(max-line-length 100).Does this pull request potentially affect one of the following parts:
@Public(Evolving): noIt is a wire format used for in-flight data exchange rather than persisted state, and the
previous encoding was already unreadable by the JVM, so the only case that changes behaviour is
a decimal field that was silently corrupted before.
work is equivalent to what avro already did; one
int.to_bytesreplaces a manual byte loop.Kubernetes/Yarn, ZooKeeper: no
Documentation
is nothing to document.