prtkgaur commented on PR #48345: URL: https://github.com/apache/arrow/pull/48345#issuecomment-5433569124
> nd accepted vector-size range appear to diverge from @wgtmac thanks — both type mismatches were real, and both are fixed on the branch. `num_exceptions` is now `uint16_t` throughout `AlpEncodedVectorInfo` (`alp.h:195`, with the constructor parameter and the accessors following it), and exception positions go through `AlpConstants::PositionType`, now `uint16_t` (`alp_constants.h:82`) with a doc comment pointing at the spec's `ExceptionPositions` field. That is the only spelling used on either the write or the read path, so the wire types for both fields match the proposed spec. I agree `num_exceptions` has to be unsigned rather than merely wanting to be: a vector at `log_vector_size = 15` holds 32768 values, which a signed 16-bit field cannot represent at all, so an all-exceptions vector was unrepresentable and not just badly encoded. The overflow you described can no longer happen — 32768 is stored as itself instead of wrapping to −32768, and the size arithmetic downstream stays positive. `AlpEdgeCaseTest.AllExceptionsAtMaxVectorSize` and `AlpCodecTest.AllExceptionsAtMaxVectorSizeRoundTrip` cover that vector directly, and a third test round-trips an exception sitting at position 32767. -- 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]
