twmb commented on PR #890:
URL: https://github.com/apache/iceberg-go/pull/890#issuecomment-4246697937
twmb/avro supports aliases on decode as well, usable via schema resolution.
The arg ordering is different in my Resolve (though in research mine matches
the ecosystem more broadly).
One "downside", maybe (it's more spec correct but less lenient), of my repo
vs. hamba is that twmb/avro is stricter when decoding writer-schema record
fields that are missing from the struct being decoded into (reader-schema) -
twmb/avro will error when trying to decode `added_data_files_count` into a
struct that doesn't have that field (i.e. the silent bug that this PR is about).
The complication with the approach proposed in the comment above is that we
need to read the ocf header to know what schema to use / what struct to decode
into, but the existing `ocf.WithReaderSchema` needs the reader schema up front
(hamba/avro has the same limitation), which means the cleanest wiring needs
either a buffer-and-reopen dance or a new API in twmb/avro.
So, I'm adding `WithReaderSchemaFunc` option to twmb/avro's ocf package (to
be released as 1.6.0 imminently, hopefully the last minor release for a while
...). The callback is invoked after the ocf header has been parsed:
`rd.Schema()` returns the writer schema, `rd.Metadata()` is populated, and
whatever schema it returns becomes the reader schema for resolution.
With that option, `ReadManifestList` becomes something like:
```go
type manifestFileV1 struct {
// ...
AddedFilesCount *int32
`avro:"added_files_count,alias=added_data_files_count"`
ExistingFilesCount *int32
`avro:"existing_files_count,alias=existing_data_files_count"`
DeletedFilesCount *int32
`avro:"deleted_files_count,alias=deleted_data_files_count"`
// ...
}
type manifestFile struct {
// ...
AddedFilesCount int32
`avro:"added_files_count,alias=added_data_files_count"`
ExistingFilesCount int32
`avro:"existing_files_count,alias=existing_data_files_count"`
DeletedFilesCount int32
`avro:"deleted_files_count,alias=deleted_data_files_count"`
// ...
}
var (
manifestFileV1Reader = avro.MustSchemaFor[manifestFileV1]()
manifestFileReader = avro.MustSchemaFor[manifestFile]()
)
func ReadManifestList(in io.Reader) ([]ManifestFile, error) {
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)
}
version = v
}
if version == 1 {
return manifestFileV1Reader, nil
}
return manifestFileReader, nil
}))
if err != nil {
return nil, err
}
switch version {
case 1:
return decodeManifestsWithFallback[*manifestFileV1](rd)
default:
return decodeManifests[*manifestFile](rd, version)
}
}
```
--
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]
