rdblue commented on code in PR #5124:
URL: https://github.com/apache/iceberg/pull/5124#discussion_r917462963
##########
python/pyiceberg/table/metadata.py:
##########
@@ -92,6 +96,14 @@ def construct_refs(cls, data: Dict[str, Any]):
data["refs"] = {MAIN_BRANCH:
SnapshotRef(snapshot_id=current_snapshot_id,
snapshot_ref_type=SnapshotRefType.BRANCH)}
return data
+ def bind(self):
Review Comment:
The main issue I have with this commit is that binding is done to transform
existing state. I think that's going to cause problems later.
For example, if I load a table and bind all of the sort orders to the
schema, what happens when I then update the schema? I think we would normally
want to be able to copy all of the sort orders into a new `TableMetadata`
object, but we would actually have to "unbind" them and rebind them to the new
schema. This in-place binding is hard to work with.
Instead, I'd recommend having separate lists of unbound and bound things.
Then when you want to get a sort order for a particular write, you can get the
unbound order, bind it to the write schema, and know it is going to work.
--
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]
