rdblue commented on code in PR #8067:
URL: https://github.com/apache/iceberg/pull/8067#discussion_r1282318939
##########
python/pyiceberg/avro/reader.py:
##########
@@ -229,11 +260,19 @@ def skip(self, decoder: BinaryDecoder) -> None:
@dataclass(frozen=True)
class DecimalReader(Reader):
+ """Reads a value as a decimal.
+
+ Decimal bytes are decoded as signed short, int or long depending on the
+ size of bytes.
+ """
+
precision: int = dataclassfield()
scale: int = dataclassfield()
def read(self, decoder: BinaryDecoder) -> Decimal:
- return decoder.read_decimal_from_bytes(self.precision, self.scale)
+ data = decoder.read(decoder.read_int())
+ unscaled_datum = int.from_bytes(data, byteorder="big", signed=True)
+ return unscaled_to_decimal(unscaled_datum, self.scale)
Review Comment:
Why move this here? The encoder still has decimal logic. I don't mind either
way whether decimal is handled by the encoder/decoder or in the reader/writer,
but it seems like we should be consistent.
Also, this reader assumes that the decimal is stored as variable-length
binary and not fixed (because it is reading the length). The spec requires that
decimals are written in Avro as fixed, length `minBytesRequired(P)`:
https://iceberg.apache.org/spec/#avro
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
