tanmayrauth commented on code in PR #890:
URL: https://github.com/apache/iceberg-go/pull/890#discussion_r3089992151
##########
manifest.go:
##########
@@ -811,43 +753,46 @@ func ReadManifest(m ManifestFile, f io.Reader,
discardDeleted bool) ([]ManifestE
// "format-version" metadata key (only manifest files are). When the key is
// absent, version 1 is assumed.
func ReadManifestList(in io.Reader) ([]ManifestFile, error) {
- rd, err := ocf.NewReader(in)
- if err != nil {
- return nil, err
- }
+ var version int
+
+ rd, err := ocf.NewReader(in, ocf.WithReaderSchemaFunc(func(rd
*ocf.Reader) (*avro.Schema, error) {
+ version = 1
+ if raw := rd.Metadata()["format-version"]; len(raw) > 0 {
+ v, err := strconv.Atoi(string(raw))
+ if err != nil {
+ return nil, fmt.Errorf("invalid format-version:
%w", err)
+ }
- sc, err := avro.Parse(string(rd.Metadata()["avro.schema"]))
- if err != nil {
- return nil, err
- }
+ version = v
+ }
- version := 1
- if raw := rd.Metadata()["format-version"]; len(raw) > 0 {
- version, err = strconv.Atoi(string(raw))
- if err != nil {
- return nil, fmt.Errorf("invalid format-version: %w",
err)
+ if version == 1 {
+ return manifestFileV1Reader, nil
}
+
+ return manifestFileReader, nil
Review Comment:
manifestFileReader already covers V2 and V3 since manifestFile includes all
fields like FirstRowIDValue. Only V1 needs a separate reader schema because its
fields use pointer types for nullable unions. Future spec versions would only
need a new branch here if they introduced fundamentally different field layouts.
--
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]
