vbhanuchander-lang commented on PR #28473:
URL: https://github.com/apache/flink/pull/28473#issuecomment-5526693574
@jubins this is correct, and I reproduced the corruption end to end rather
than just reading it —
posting the evidence so a committer has less to redo, since the PR is
already `community-reviewed`.
Pickled a real `array('l', ...)` in Python and unpickled it through
`pyrolite 4.13` (Flink's actual
dependency, `flink-python/pom.xml:177`) with your class and with `master`'s,
verbatim:
```
python: array('l', [3000000000, Long.MAX_VALUE, 0, -1])
master (args[0] == "l") -> int[] [-1294967296, -1, 0, -1]
PR ("l".equals(...)) -> long[] [3000000000, 9223372036854775807, 0,
-1]
```
So it is not only truncation — `3000000000` comes back as `-1294967296` and
`Long.MAX_VALUE` as
`-1`. Wrong values, sign flipped, no error raised.
**Why the branch can never be taken**, which is the part worth recording on
the issue: the typecode
arrives as a `BINUNICODE` from the pickle stream — visible as `58 01 00 00
00 6c` in the bytes above
— so the unpickler constructs a fresh `String`. It is never the interned
`"l"` literal that
`==` compares against. Confirmed directly: with the deserialized value,
`args[0] == "l"` is `false`
while `"l".equals(args[0])` is `true`. The `long[]` path has therefore been
dead code, and every
typecode `'l'` array has been falling through to the superclass, which
builds an `int[]`.
Your test uses `new String("l")` for exactly this reason, and that is the
right call — an interned
literal would make it pass against the unfixed code.
@dianfu would you be willing to merge this one? It is a one-character fix in
`flink-python` with a
test, already carrying `community-reviewed`, and the failure mode is silent
wrong data rather than
an exception. Full disclosure on my method: I copied Flink's
`ArrayConstructor` verbatim into a
scratch class against the real `pyrolite` jar rather than building Flink, so
the numbers above are
from that harness, not from the project's test suite.
--
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]
