rdblue commented on code in PR #40:
URL: https://github.com/apache/iceberg-python/pull/40#discussion_r1349763567
##########
pyiceberg/avro/writer.py:
##########
@@ -49,58 +51,69 @@ def __repr__(self) -> str:
return f"{self.__class__.__name__}()"
+@dataclass(frozen=True)
class NoneWriter(Writer):
- def write(self, _: BinaryEncoder, __: Any) -> None:
- pass
+ def write(self, encoder: BinaryEncoder, __: Any) -> None:
+ encoder.write_int(0)
Review Comment:
I don't think this is correct. Avro encodes null as the null type, which has
no byte representation. The only way to encode null is to select it as a branch
of a union. But `null` in an Avro union can be anywhere.
I think the previous version was correct, where the NoneWriter skipped
writing any bytes. That corresponds to writing a `null` value in Avro, where
the containing option writer actually selects the null branch of the union.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
