rdblue commented on code in PR #4685:
URL: https://github.com/apache/iceberg/pull/4685#discussion_r868705720
##########
python/src/iceberg/schema.py:
##########
@@ -262,10 +267,45 @@ def primitive(self, primitive: PrimitiveType) -> T:
... # pragma: no cover
+@dataclass(init=True, eq=True)
+class Accessor:
+ """An accessor for a specific position in a container that implements the
StructProtocol"""
+
+ position: int
+ inner: Optional["Accessor"] = None
+
+ def __str__(self):
+ return f"Accessor(position={self.position},inner={self.inner})"
+
+ def __repr__(self):
+ return self.__str__()
+
+ def with_inner(self, accessor: "Accessor") -> "Accessor":
+ parent = copy.deepcopy(self)
+ acc = parent
+ while acc.inner:
+ acc = acc.inner
+ acc.inner = accessor
+ return parent
+
+ def get(self, container: StructProtocol) -> Any:
+ """Returns the value at self.position in `container`
+
+ Args:
+ container(StructProtocol): A container to access at position
`self.position`
+
+ Returns:
+ Any: The value at position `self.position` in the container
+ """
+ return container.get(self.position)
+
+
@singledispatch
def visit(obj, visitor: SchemaVisitor[T]) -> T:
"""A generic function for applying a schema visitor to any point within a
schema
+ The function traverses the schema in pre-order fashion
Review Comment:
The purpose of the visitor pattern is to separate the logic that traverses
the tree structure from the logic that operates on it. Moving to pre-order
conflicts with that goal because you have to inspect the inner type using
`instanceof`. It think it looks more like recursive method that traverses the
schema because it has to redo the work of the visitor.
Iceberg uses the pattern of using a visitor to build a tree structure quite
a bit, and to do that you almost always want to use post-order so that you can
wrap what was returned by visiting the children of a node. There are other ways
to do it, but you end up keeping state in the visitor or needing to pass state
down the tree. I think it's cleaner and easier to work with using the existing
pattern. But even if that's not convincing, I'm concerned about having to
rewrite all of the Java visitors to use different logic, rather than being able
to see what they're doing and port the same logic. That's going to be quite a
bit harder.
--
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]
