zeroshade commented on code in PR #317:
URL: https://github.com/apache/iceberg-go/pull/317#discussion_r1985407723
##########
manifest.go:
##########
@@ -1519,7 +1492,13 @@ func (f *fallbackManifestEntryV1) toEntry()
*manifestEntryV1 {
return &f.manifestEntryV1
}
-func (m *manifestEntryV1) inheritSeqNum(manifest ManifestFile) {}
+func (m *manifestEntryV1) inherit(manifest ManifestFile) {
+ if m.Snapshot <= 0 {
+ m.Snapshot = manifest.SnapshotID()
+ }
Review Comment:
So currently the only reason we have separate objects to represent V1 and V2
manifests and entries (as opposed to the way Pyiceberg does it) is to leverage
the ease of the Avro handling via the struct tags for ser/de. The differences
in the schemas for V1/V2 means we need separate objects to handle the
nullability and the Avro lib we're using here doesn't provide a good way to
customize the encoding/decoding at a member level like that.
That all said, it might be time to refactor this entire thing so that the
ultimate object the consumer interacts with is consolidated and the differences
are only handled at the point of encoding and decoding, to better generalize
all the other operations.
I'll take a stab at it today and see how bad it is.
As far as "so called fast appends", what do you mean?
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
