Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdks/python/apache_beam/coders/coder_impl.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ cdef class TimestampCoderImpl(StreamCoderImpl):

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


Expand Down
34 changes: 8 additions & 26 deletions sdks/python/apache_beam/coders/coder_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,6 @@
is_compiled = False
fits_in_64_bits = lambda x: -(1 << 63) <= x <= (1 << 63) - 1


def _as_signed_int64(value):
# type: (int) -> int

"""Folds a uint64 to the signed int64 with the same bits (and VarInt
encoding), which the Cython int64_t stream params accept. Values outside the
64-bit range raise here so the pure-Python path matches Cython's overflow."""
if (1 << 63) <= value < (1 << 64):
return int(value) - (1 << 64)
if not fits_in_64_bits(value):
raise OverflowError("%d is out of range for a 64-bit integer." % value)
return value


if TYPE_CHECKING or SLOW_STREAM:
from .slow_stream import ByteCountingOutputStream
from .slow_stream import InputStream as create_InputStream
Expand Down Expand Up @@ -1058,26 +1044,22 @@ class VarIntCoderImpl(StreamCoderImpl):
def encode_to_stream(self, value, out, nested):
# type: (int, create_OutputStream, bool) -> None
try:
# Fold uint64 values into signed int64 so Cython doesn't overflow.
out.write_var_int64(_as_signed_int64(value))
out.write_var_int64(value)
except OverflowError as e:
raise OverflowError(
f"Integer value '{value}' is out of the encodable range for "
f"VarIntCoder. This coder is limited to 64-bit integers: the "
f"signed range -(2**63) to 2**63 - 1, plus unsigned values up to "
f"2**64 - 1 which share the same wire encoding. "
f"VarIntCoder. This coder is limited to values that fit "
f"within a 64-bit signed integer (-(2**63) to 2**63 - 1). "
f"Original error: {e}") from e

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

def encode(self, value):
# Compare as a Python object: a uint64 value overflows the int64_t cast
# the compiled fast path used to do here. Non-small values (including
# uint64) fall through to encode_to_stream, which folds them.
if 0 <= value < len(small_ints):
return small_ints[value]
ivalue = value # type cast
if 0 <= ivalue < len(small_ints):
return small_ints[ivalue]
return StreamCoderImpl.encode(self, value)
Comment on lines 1059 to 1063

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Cython compiled mode, ivalue is declared as int64_t in .pxd. The assignment ivalue = value performs a type cast. If value is an integer that overflows the 64-bit signed integer range, this assignment will raise an OverflowError immediately.

This bypasses the fallback to StreamCoderImpl.encode(self, value) and the custom OverflowError handling in encode_to_stream (lines 1048-1053), which is designed to provide a helpful error message explaining the 64-bit signed integer limit. Instead, the user receives a generic Cython OverflowError.

Wrapping the type cast and small integer check in a try...except OverflowError block allows the overflow to be caught and safely delegated to StreamCoderImpl.encode, which will then raise the descriptive error message.

  def encode(self, value):
    try:
      ivalue = value  # type cast
      if 0 <= ivalue < len(small_ints):
        return small_ints[ivalue]
    except OverflowError:
      pass
    return StreamCoderImpl.encode(self, value)


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


Expand Down
18 changes: 0 additions & 18 deletions sdks/python/apache_beam/coders/coders_test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,24 +437,6 @@ def test_varint_coder(self):
for k in range(0, int(math.log(MAX_64_BIT_INT)))
])

def test_varint_coder_uint64(self):
# uint64 values [2**63, 2**64) must encode like the signed int64 with the
# same bits instead of overflowing Cython's int64_t. Decoding is signed,
# matching Java's VarIntCoder.
coder = coders.VarIntCoder()
impl = coder.get_impl()
for v in [1 << 63, (1 << 63) + 12345, (1 << 64) - 1]:
signed_twin = v - (1 << 64)
encoded = coder.encode(v)
self.assertEqual(encoded, coder.encode(signed_twin))
self.assertEqual(impl.estimate_size(v), len(encoded))
self.assertEqual(coder.decode(encoded), signed_twin)

# Values outside the 64-bit range stay out of range on both paths.
for v in [1 << 64, (1 << 70), -(1 << 63) - 1]:
with self.assertRaises(OverflowError):
coder.encode(v)

def test_varint32_coder(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since test_varint_coder_uint64 is being removed, we should ensure that we still have test coverage verifying that values outside the supported 64-bit signed integer range (such as 1 << 63, 1 << 64, and -(1 << 63) - 1) correctly raise OverflowError.

Suggested change
def test_varint32_coder(self):
def test_varint_coder_overflow(self):
coder = coders.VarIntCoder()
for v in [1 << 63, 1 << 64, -(1 << 63) - 1]:
with self.assertRaises(OverflowError):
coder.encode(v)
def test_varint32_coder(self):

# Small ints.
self.check_coder(coders.VarInt32Coder(), *range(-10, 10))
Expand Down
Loading