wroever opened a new pull request, #1984: URL: https://github.com/apache/iceberg-go/pull/1984
Closes #1983, which has the full write-up and reproduction. ## Problem The Avro manifest decoder assigns the wire `file_format` string directly into the typed `FileFormat` field, whose constants are uppercase: ```go // manifest.go Format FileFormat `avro:"file_format"` ``` `GetFile` (`table/internal/interfaces.go`) then matches the decoded value against `iceberg.ParquetFile` (`"PARQUET"`) to pick a reader. A manifest written with lowercase `parquet` never matches, so the scan fails on the first data file — with a message that renders both spellings identically: ``` not implemented: only parquet format is implemented, got parquet ``` Nothing unusual is needed to hit this: `LoadTable` followed by a bare `tbl.Scan()`. DuckDB's `iceberg` extension writes lowercase, so no table it has written is readable. It also isn't limited to explicit scans — `Transaction.Delete` runs a copy-on-write rewrite through the same path, so appends and deletes against such a table fail too. ## Fix Route the decoded value through the existing, already case-insensitive `FileFormatFromString` in `ManifestReader.ReadEntry`, which covers both the normal and fallback entry paths (they share the same `*dataFile`), and normalize before the existing status/content validation. This mirrors what the other implementations do on this exact path — Java's `BaseFile.internalSet` → `FileFormat.fromString`, and PyIceberg's `FileFormat._missing_` — so it brings Go in line rather than introducing new leniency. It also makes the manifest reader consistent with the rest of this library, which already normalizes everywhere else this field crosses the wire: - `catalog/rest/scan_task_decoder.go` — the REST scan-planning decoder calls `FileFormatFromString` before building the data file. - `table/writer.go` and `table/rolling_data_writer.go` — both writers normalize. The Avro manifest reader was the only place treating wire casing as authoritative. ## Note for reviewers An unrecognized `file_format` is now a hard error at decode time, naming the offending file path, rather than surfacing later as a confusing reader-selection failure. This matches Java, where `FileFormat.fromString` throws. The lenient alternative — pass unknown values through untouched and let file-open fail — is a one-line change if reviewers prefer it. ## Testing Adds `TestManifestReaderNormalizesFileFormat`, which writes a real v2 manifest with `file_format` spelled `parquet`, `Parquet`, `PARQUET`, `avro`, and `csv`, then asserts the reader returns the uppercase constant for the first four and errors on the last. -- 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]
