Skip to content

Commit e87f50e

Browse files
AviKndrclaude
andauthored
Fix Python VarIntCoder OverflowError on uint64 values (#39047)
* [BEAM] Fix Python VarIntCoder OverflowError on uint64 values The Cython write_var_int64/get_varint_size stream methods take a signed int64_t parameter. A Python int in the unsigned 64-bit range [2**63, 2**64) -- a uint64 -- is converted to that signed parameter at the call boundary and rejected with an OverflowError before the method body runs, even though the body already operates on the unsigned bit pattern and the VarInt wire encoding is well-defined. Fold such values to the signed int64 with the identical bit pattern (and thus identical VarInt encoding) before handing them to the stream. This matches Java's signed VarIntCoder on the wire; decoding remains signed. Values past 64 bits are left unchanged and still overflow downstream, preserving the coder's documented 64-bit limit. Adds test_varint_coder_uint64 covering no-overflow encoding, wire equivalence to the signed twin, size estimation, signed decode, and the still-raising out-of-range case (guarded on the compiled implementation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Enforce 64-bit range in _as_signed_int64 for path parity Address review feedback: raise OverflowError in _as_signed_int64 for values outside the 64-bit range (reusing fits_in_64_bits) so the pure-Python path matches the Cython int64_t overflow instead of silently encoding out-of-range values. Both encode_to_stream and estimate_size already wrap OverflowError, so the user-facing message is unchanged. Drop the is_compiled guard in test_varint_coder_uint64 so the out-of-range case runs on both paths, add the negative lower-bound case (-(2**63) - 1), and remove the now-unused coder_impl import. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix uint64 overflow in VarIntCoderImpl.encode fast path The compiled encode() cast value to a C int64_t (ivalue, typed via coder_impl.pxd) for the small-ints fast path, which overflowed on uint64 values before reaching encode_to_stream's fold -- the cause of the test_varint_coder_uint64 CI failure on the Python PreCommit suites. Do the small-ints check with a plain Python-object comparison so it can't overflow; non-small values (including uint64 and out-of-range ints) fall through to StreamCoderImpl.encode -> encode_to_stream, where _as_signed_int64 folds them and raises the wrapped OverflowError for genuinely out-of-range values. Drop the now-unused ivalue int64_t local hint from coder_impl.pxd. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f08c259 commit e87f50e

3 files changed

Lines changed: 44 additions & 9 deletions

File tree

sdks/python/apache_beam/coders/coder_impl.pxd

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ cdef class TimestampCoderImpl(StreamCoderImpl):
135135

136136
cdef list small_ints
137137
cdef class VarIntCoderImpl(StreamCoderImpl):
138-
@cython.locals(ivalue=libc.stdint.int64_t)
139138
cpdef bytes encode(self, value)
140139

141140

sdks/python/apache_beam/coders/coder_impl.py

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,20 @@
9090
is_compiled = False
9191
fits_in_64_bits = lambda x: -(1 << 63) <= x <= (1 << 63) - 1
9292

93+
94+
def _as_signed_int64(value):
95+
# type: (int) -> int
96+
97+
"""Folds a uint64 to the signed int64 with the same bits (and VarInt
98+
encoding), which the Cython int64_t stream params accept. Values outside the
99+
64-bit range raise here so the pure-Python path matches Cython's overflow."""
100+
if (1 << 63) <= value < (1 << 64):
101+
return int(value) - (1 << 64)
102+
if not fits_in_64_bits(value):
103+
raise OverflowError("%d is out of range for a 64-bit integer." % value)
104+
return value
105+
106+
93107
if TYPE_CHECKING or SLOW_STREAM:
94108
from .slow_stream import ByteCountingOutputStream
95109
from .slow_stream import InputStream as create_InputStream
@@ -1044,22 +1058,26 @@ class VarIntCoderImpl(StreamCoderImpl):
10441058
def encode_to_stream(self, value, out, nested):
10451059
# type: (int, create_OutputStream, bool) -> None
10461060
try:
1047-
out.write_var_int64(value)
1061+
# Fold uint64 values into signed int64 so Cython doesn't overflow.
1062+
out.write_var_int64(_as_signed_int64(value))
10481063
except OverflowError as e:
10491064
raise OverflowError(
10501065
f"Integer value '{value}' is out of the encodable range for "
1051-
f"VarIntCoder. This coder is limited to values that fit "
1052-
f"within a 64-bit signed integer (-(2**63) to 2**63 - 1). "
1066+
f"VarIntCoder. This coder is limited to 64-bit integers: the "
1067+
f"signed range -(2**63) to 2**63 - 1, plus unsigned values up to "
1068+
f"2**64 - 1 which share the same wire encoding. "
10531069
f"Original error: {e}") from e
10541070

10551071
def decode_from_stream(self, in_stream, nested):
10561072
# type: (create_InputStream, bool) -> int
10571073
return in_stream.read_var_int64()
10581074

10591075
def encode(self, value):
1060-
ivalue = value # type cast
1061-
if 0 <= ivalue < len(small_ints):
1062-
return small_ints[ivalue]
1076+
# Compare as a Python object: a uint64 value overflows the int64_t cast
1077+
# the compiled fast path used to do here. Non-small values (including
1078+
# uint64) fall through to encode_to_stream, which folds them.
1079+
if 0 <= value < len(small_ints):
1080+
return small_ints[value]
10631081
return StreamCoderImpl.encode(self, value)
10641082

10651083
def decode(self, encoded):
@@ -1073,11 +1091,11 @@ def estimate_size(self, value, nested=False):
10731091
# type: (Any, bool) -> int
10741092
# Note that VarInts are encoded the same way regardless of nesting.
10751093
try:
1076-
return get_varint_size(value)
1094+
return get_varint_size(_as_signed_int64(value))
10771095
except OverflowError as e:
10781096
raise OverflowError(
10791097
f"Cannot estimate size for integer value '{value}'. "
1080-
f"Value is out of the range for VarIntCoder (64-bit signed integer). "
1098+
f"Value is out of the range for VarIntCoder (64-bit integer). "
10811099
f"Original error: {e}") from e
10821100

10831101

sdks/python/apache_beam/coders/coders_test_common.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,24 @@ def test_varint_coder(self):
437437
for k in range(0, int(math.log(MAX_64_BIT_INT)))
438438
])
439439

440+
def test_varint_coder_uint64(self):
441+
# uint64 values [2**63, 2**64) must encode like the signed int64 with the
442+
# same bits instead of overflowing Cython's int64_t. Decoding is signed,
443+
# matching Java's VarIntCoder.
444+
coder = coders.VarIntCoder()
445+
impl = coder.get_impl()
446+
for v in [1 << 63, (1 << 63) + 12345, (1 << 64) - 1]:
447+
signed_twin = v - (1 << 64)
448+
encoded = coder.encode(v)
449+
self.assertEqual(encoded, coder.encode(signed_twin))
450+
self.assertEqual(impl.estimate_size(v), len(encoded))
451+
self.assertEqual(coder.decode(encoded), signed_twin)
452+
453+
# Values outside the 64-bit range stay out of range on both paths.
454+
for v in [1 << 64, (1 << 70), -(1 << 63) - 1]:
455+
with self.assertRaises(OverflowError):
456+
coder.encode(v)
457+
440458
def test_varint32_coder(self):
441459
# Small ints.
442460
self.check_coder(coders.VarInt32Coder(), *range(-10, 10))

0 commit comments

Comments
 (0)