AmirF194 opened a new pull request, #39943:
URL: https://github.com/apache/beam/pull/39943
`PickleCoder.as_deterministic_coder()` and
`_MemoizingPickleCoder.as_deterministic_coder()` both construct
`FastPrimitivesCoder` with a `requires_deterministic` keyword argument:
```python
def as_deterministic_coder(self, step_label, error_message=None):
return FastPrimitivesCoder(self, requires_deterministic=step_label)
```
`FastPrimitivesCoder.__init__(self, fallback_coder=PickleCoder())` has never
accepted that argument, so both raise `TypeError` unconditionally. The bug goes
back to commit 9be70c9861, which changed `PickleCoder.as_deterministic_coder`
from the working `DeterministicFastPrimitivesCoder(self, step_label)` to this
broken call, in the same diff that added a `requires_deterministic` kwarg to a
different class (`coder_impl.FastPrimitivesCoderImpl`, the Cython impl, not the
`coders.FastPrimitivesCoder` wrapper referenced here). `_MemoizingPickleCoder`
later copied the already-broken line.
The invariant `as_deterministic_coder()` is supposed to hold, that it always
returns a working deterministic coder rather than raising, is exactly what
`FastPrimitivesCoder`'s own `as_deterministic_coder` already does by delegating
to `_update_compatible_deterministic_fast_primitives_coder`. This PR routes
both call sites through that same helper instead.
A reachable consumer: `GroupByEncryptedKey.expand()` calls
`coder.as_deterministic_coder(...)` inside `except ValueError`, expecting a
non-deterministic key to log a warning. When the coder resolves to
`PickleCoder`/`_MemoizingPickleCoder`, the uncaught `TypeError` crashes
pipeline construction instead.
Verified:
- `PickleCoder().as_deterministic_coder('x')` and
`_MemoizingPickleCoder().as_deterministic_coder('x')` raise `TypeError` on
`master`, confirmed live against the installed `apache-beam` package
(byte-identical `coders.py`) and against this branch.
- New `PickleCoderTest.test_as_deterministic_coder` fails on `master`,
passes on this branch, both runs in the same container.
- `coders_test.py`, `typecoders_test.py`, and `fast_coders_test.py` (the
standard-coder correctness suite, all classes including
`FastPrimitivesCoder`/`DeterministicFastPrimitivesCoder`) pass unchanged.
- `ruff check` and `yapf --diff` are clean on both changed files.
- Not checked: the Cython (`coder_impl`) fast paths, since this fix is
confined to the plain-Python `coders.py` wrapper layer and does not touch
`coder_impl`.
Fixes #39942
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
