Fokko commented on code in PR #4119:
URL: https://github.com/apache/iceberg-python/pull/4119#discussion_r4306812730
##########
pyiceberg/utils/schema_conversion.py:
##########
@@ -81,7 +81,15 @@
AvroType = str | Any
+# Avro types that have a name and can be referred to by it later in the schema
+AVRO_NAMED_TYPES = {"record", "enum", "fixed"}
Review Comment:
Let's use a `frozenset` here
##########
pyiceberg/utils/schema_conversion.py:
##########
@@ -505,6 +522,18 @@ def __init__(self, schema_name: str | None) -> None:
schema_name: The name of the root record.
"""
self.schema_name = schema_name
+ self.defined_names: set[str] = set()
+
+ def _define_once(self, named_type: dict[str, Any]) -> AvroType:
+ """Return the definition of a named type the first time, and a
reference to its name after that.
+
+ Avro allows only one definition per name; later uses must refer to it
by name.
+ """
+ name = named_type["name"]
+ if name in self.defined_names:
+ return name
+ self.defined_names.add(name)
+ return named_type
Review Comment:
I think we're doing quite a bit of additional bookkeeping. I'm wondering if
it wouldn't be easier to include the field-id to the name (which is unique by
definition).
--
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]
