dwilson1988 commented on code in PR #177:
URL: https://github.com/apache/iceberg-go/pull/177#discussion_r1813441342


##########
manifest.go:
##########
@@ -831,14 +946,53 @@ func (m *manifestEntryV1) FileSequenceNum() *int64 {
        return m.FileSeqNum
 }
 
-func (m *manifestEntryV1) DataFile() DataFile { return &m.Data }
+func (m *manifestEntryV1) DataFile() DataFile { return m.Data }
+
+// ManifestEntryV2Builder is a helper for building a V2 manifest entry
+// struct which will conform to the ManifestEntry interface.
+type ManifestEntryV2Builder struct {
+       m *manifestEntryV2
+}
+
+// NewManifestEntryV2Builder is passed all of the required fields and then 
allows
+// all of the optional fields to be set by calling the corresponding methods
+// before calling [ManifestEntryV2Builder.Build] to construct the object.
+func NewManifestEntryV2Builder(status ManifestEntryStatus, snapshotID int64, 
data DataFile) *ManifestEntryV2Builder {
+       return &ManifestEntryV2Builder{
+               m: &manifestEntryV2{
+                       EntryStatus: status,
+                       Snapshot:    &snapshotID,
+                       Data:        data,

Review Comment:
   Happy to make that change, but there are two additional concerns that raises:
   1. Not a big deal, but leaving the type as `dataFile` means copying a mutex 
(`sync.Once`). This shouldn't be an issue in the current usage, but something 
to consider if you want to avoid that potential issue.
   2. Since `NewManifestEntryVXBuilder` accepts an interface type and is 
exported, a user could potentially provide another type that fullfils the 
`DataFile` interface. I can sidestep this by changing the signature of the 
`NewManifestVXBuilder` to return an error on a failed type assertion, but just 
wanted to bring this up since it muddies the exported footprint. However, 
accepting and using the interface type means a type that is not avro-encodable 
will result in a runtime error, so maybe it's a moot point.



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

Reply via email to