Fokko commented on code in PR #4685:
URL: https://github.com/apache/iceberg/pull/4685#discussion_r865144092


##########
python/src/iceberg/schema.py:
##########
@@ -290,6 +292,8 @@ def _(obj: StructType, visitor: SchemaVisitor[T]) -> T:
         visitor.before_field(field)
         try:
             result = visit(field.type, visitor)
+        except Exception:
+            logger.exception("Unable to visit the field in the schema")

Review Comment:
   Great idea! I'm actually not sure if we should swallow those exceptions.



##########
python/src/iceberg/expressions/base.py:
##########
@@ -278,6 +278,9 @@ def __str__(self):
     def __repr__(self):
         return f"Accessor(position={self._position})"
 
+    def __eq__(self, other):
+        return other and self._position == other.position

Review Comment:
   We only use it for tests right now, but that's a good point! Thanks!



##########
python/src/iceberg/schema.py:
##########
@@ -503,21 +507,34 @@ class _BuildPositionAccessors(SchemaVisitor[Dict[int, 
"Accessor"]]):
     def __init__(self) -> None:
         self._index: Dict[int, Accessor] = {}
 
-    def schema(self, schema, result: Dict[int, Accessor]) -> Dict[int, 
Accessor]:
-        return self._index
+    def schema(self, schema: Schema, field_results: Dict[int, Accessor]) -> 
Dict[int, Accessor]:
+        return field_results
+
+    def struct(self, struct: StructType, field_results: List[Dict[int, 
Accessor]]) -> Dict[int, Accessor]:
+        fields = struct.fields
+        for idx, field in enumerate(fields):
+            result = field_results[idx]
+            if result:
+                self._index.update(result)
+            else:
+                self._index[field.field_id] = Accessor(field.field_id)

Review Comment:
   Thanks @rdblue the example is very helpful. I've added it to the docstring 
as well 👍🏻 



-- 
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]

Reply via email to