s3tables: accept Puffin statistics files in a table's metadata/ (#11892)

Table and partition statistics are Puffin files that the table metadata lists
in its statistics entries; orphan cleanup keeps them (#11651), and its test
places one at metadata/stats-1.puffin. The table bucket layout rule accepted
only *.stats among them, so an engine that names a statistics file *.puffin
had the upload refused with 403 and could not write statistics at all.

The metadata patterns now admit a *.puffin file directly under metadata/,
with the same safe-character rule as the other catch-all patterns. Nothing
else widens: a Puffin file in a subdirectory of metadata/, with a trailing
extension, or outside <namespace>/<table>/ is still refused.

Test: the new metadata/*.puffin cases in
TestIcebergLayoutValidator_AcceptsRealWorldManifestNames and
TestTableBucketFileValidator_PuffinStatisticsOnlyInTableMetadata fail on
master ("invalid metadata file format") and pass with the change; the
rejected cases pass on both.

Co-authored-by: Javier de la Torre <jatorre@lakehousebox.com>
This commit is contained in:
Javier de la Torre
2026-10-21 12:19:53 -07:00
committed by GitHub
co-authored by Javier de la Torre
parent 03065155db
commit 593cfee85b
2 changed files with 38 additions and 0 deletions
+4
View File
@@ -15,6 +15,7 @@ import (
// - metadata/v*.metadata.json (table metadata)
// - metadata/snap-*.avro (snapshot manifest lists)
// - metadata/*.avro (manifest files)
// - metadata/*.stats, metadata/*.puffin (statistics files)
// - data/*.parquet, *.orc, *.avro, *.puffin (data files)
const uuidPattern = `[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}`
@@ -68,6 +69,9 @@ var (
// whose naming we can't anticipate across engines and versions.
regexp.MustCompile(`^[A-Za-z0-9._-]+\.avro$`),
regexp.MustCompile(`^[A-Za-z0-9._-]+\.metadata\.json$`),
// Table and partition statistics files listed in the table metadata
// are Puffin files; engines name them *.stats or *.puffin.
regexp.MustCompile(`^[A-Za-z0-9._-]+\.puffin$`),
}
// Patterns for valid data files
@@ -45,6 +45,10 @@ func TestIcebergLayoutValidator_AcceptsRealWorldManifestNames(t *testing.T) {
"trino/iceberg stats file",
"metadata/table.stats",
},
{
"puffin statistics file",
"metadata/20260928-1-0199a3c2-7d1e-7c55-9a0e-3b1f2c4d5e6f.puffin",
},
}
v := NewIcebergLayoutValidator()
@@ -70,6 +74,8 @@ func TestIcebergLayoutValidator_RejectsClearlyBadMetadataNames(t *testing.T) {
{"subdirectory under metadata is not allowed", "metadata/sub/file.avro"},
{"top-level dir other than metadata or data", "garbage/file.avro"},
{"catch-all anchors must reject trailing extension", "metadata/file.avro.txt"},
{"puffin with a trailing extension", "metadata/stats.puffin.txt"},
{"puffin in a subdirectory under metadata", "metadata/sub/stats.puffin"},
}
v := NewIcebergLayoutValidator()
@@ -137,3 +143,31 @@ func TestValidateFilePathAcceptsLanceLayout(t *testing.T) {
}
}
}
// Statistics files are referenced from the table metadata and kept by orphan
// cleanup; the upload door has to admit them in the table's metadata/, and
// only there.
func TestTableBucketFileValidator_PuffinStatisticsOnlyInTableMetadata(t *testing.T) {
v := NewTableBucketFileValidator()
accepted := []string{
TablesPath + "/cat/ns/t/metadata/stats-1.puffin",
TablesPath + "/cat/ns/t/metadata/table.stats",
TablesPath + "/cat/ns/t/data/0199a3c2-deletes.puffin",
}
for _, p := range accepted {
if err := v.ValidateTableBucketUpload(p); err != nil {
t.Errorf("ValidateTableBucketUpload(%q) = %v, want nil", p, err)
}
}
rejected := []string{
TablesPath + "/cat/ns/t/stats-1.puffin",
TablesPath + "/cat/ns/t/other/stats-1.puffin",
TablesPath + "/cat/ns/t/metadata/sub/stats-1.puffin",
TablesPath + "/cat/ns/t/metadata/notes.txt",
}
for _, p := range rejected {
if err := v.ValidateTableBucketUpload(p); err == nil {
t.Errorf("ValidateTableBucketUpload(%q) = nil, want an error", p)
}
}
}