Fokko commented on code in PR #4685:
URL: https://github.com/apache/iceberg/pull/4685#discussion_r868346289
##########
python/src/iceberg/schema.py:
##########
@@ -279,53 +319,74 @@ def visit(obj, visitor: SchemaVisitor[T]) -> T:
@visit.register(Schema)
def _(obj: Schema, visitor: SchemaVisitor[T]) -> T:
"""Visit a Schema with a concrete SchemaVisitor"""
- return visitor.schema(obj, visit(obj.as_struct(), visitor))
+ return visit(obj.as_struct(), visitor)
@visit.register(StructType)
def _(obj: StructType, visitor: SchemaVisitor[T]) -> T:
"""Visit a StructType with a concrete SchemaVisitor"""
- results = []
+
+ result = visitor.struct(obj)
+
for field in obj.fields:
+ visitor.field(field)
visitor.before_field(field)
try:
- result = visit(field.type, visitor)
+ visit(field.type, visitor)
+ except Exception as e:
+ logger.exception(f"Unable to visit the field: {field}")
+ raise e
finally:
visitor.after_field(field)
- results.append(visitor.field(field, result))
-
- return visitor.struct(obj, results)
+ return result
@visit.register(ListType)
def _(obj: ListType, visitor: SchemaVisitor[T]) -> T:
"""Visit a ListType with a concrete SchemaVisitor"""
+ result = visitor.list(obj)
+
+ visitor.field(obj.element)
+
visitor.before_list_element(obj.element)
try:
- result = visit(obj.element.type, visitor)
+ visit(obj.element.type, visitor)
+ except Exception as e:
+ logger.exception(f"Unable to visit the type: {obj}")
+ raise e
finally:
visitor.after_list_element(obj.element)
- return visitor.list(obj, result)
+ return result
@visit.register(MapType)
def _(obj: MapType, visitor: SchemaVisitor[T]) -> T:
"""Visit a MapType with a concrete SchemaVisitor"""
+ result = visitor.map(obj)
+
+ visitor.field(obj.key)
visitor.before_map_key(obj.key)
try:
- key_result = visit(obj.key.type, visitor)
+ visit(obj.key.type, visitor)
+ except Exception as e:
+ logger.exception(f"Unable to visit the map ket type: {obj.key}")
+ raise e
finally:
visitor.after_map_key(obj.key)
+ visitor.field(obj.value)
visitor.before_map_value(obj.value)
try:
- value_result = visit(obj.value.type, visitor)
+ visit(obj.value.type, visitor)
+ except Exception as e:
+ logger.exception(f"Unable to visit the map value type: {obj.value}")
Review Comment:
This was in the original code, but it would be best to remove this. If
something fails while traversing the schema, we don't want to continue and just
return a part of the schema. In that case we just want to throw an exception
--
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]
