You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by at...@apache.org on 2026/10/15 18:21:58 UTC
(iceberg-go) branch main updated: fix(table): preserve local file paths (#2175)
This is an automated email from the ASF dual-hosted git repository.
laskoviymishka pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 4fdbf65ef fix(table): preserve local file paths (#2175)
4fdbf65ef is described below
commit 4fdbf65ef234b8715dbd2459d2851b7c6f576257
Author: Mohamed Arbi <mo...@gmail.com>
AuthorDate: Thu Oct 15 19:21:53 2026 +0100
fix(table): preserve local file paths (#2175)
---
table/locations.go | 72 +++++++++++----------------------
table/locations_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 128 insertions(+), 48 deletions(-)
diff --git a/table/locations.go b/table/locations.go
index c181c968a..74cb3dcfd 100644
--- a/table/locations.go
+++ b/table/locations.go
@@ -19,8 +19,8 @@ package table
import (
"fmt"
- "net/url"
"path"
+ "strings"
"github.com/apache/iceberg-go"
"github.com/google/uuid"
@@ -44,14 +44,20 @@ type LocationProvider interface {
}
type simpleLocationProvider struct {
- tableLoc *url.URL
tableProps iceberg.Properties
- dataPath *url.URL
- metadataPath *url.URL
+ dataPath string
+ metadataPath string
+}
+
+// joinLocationPath appends components using forward slashes.
+// It trims trailing slashes from the base but does not otherwise clean,
+// encode, or decode the local path or URI.
+func joinLocationPath(location string, elem ...string) string {
+ return strings.TrimRight(location, "/") + "/" + strings.Join(elem, "/")
}
func (slp *simpleLocationProvider) NewDataLocation(dataFileName string) string {
- return slp.dataPath.JoinPath(dataFileName).String()
+ return joinLocationPath(slp.dataPath, dataFileName)
}
func (slp *simpleLocationProvider) NewTableMetadataFileLocation(newVersion int) (string, error) {
@@ -84,35 +90,15 @@ func (slp *simpleLocationProvider) NewTableMetadataFileLocation(newVersion int)
}
func (slp *simpleLocationProvider) NewMetadataLocation(metadataFileName string) string {
- return slp.metadataPath.JoinPath(metadataFileName).String()
+ return joinLocationPath(slp.metadataPath, metadataFileName)
}
-func newSimpleLocationProvider(tableLoc *url.URL, tableProps iceberg.Properties) (*simpleLocationProvider, error) {
- out := &simpleLocationProvider{
- tableLoc: tableLoc,
- tableProps: tableProps,
- }
-
- var err error
- if propPath, ok := tableProps[WriteDataPathKey]; ok {
- out.dataPath, err = url.Parse(propPath)
- if err != nil {
- return nil, err
- }
- } else {
- out.dataPath = out.tableLoc.JoinPath("data")
- }
-
- if propPath, ok := tableProps[WriteMetadataPathKey]; ok {
- out.metadataPath, err = url.Parse(propPath)
- if err != nil {
- return nil, err
- }
- } else {
- out.metadataPath = out.tableLoc.JoinPath("metadata")
+func newSimpleLocationProvider(tableLoc string, tableProps iceberg.Properties) *simpleLocationProvider {
+ return &simpleLocationProvider{
+ tableProps: tableProps,
+ dataPath: tableProps.Get(WriteDataPathKey, joinLocationPath(tableLoc, "data")),
+ metadataPath: tableProps.Get(WriteMetadataPathKey, joinLocationPath(tableLoc, "metadata")),
}
-
- return out, nil
}
type objectStoreLocationProvider struct {
@@ -149,34 +135,24 @@ func (p *objectStoreLocationProvider) NewDataLocation(dataFileName string) strin
hashedPath := computeHash(dataFileName)
if p.includePartitionPaths {
- return p.simpleLocationProvider.dataPath.JoinPath(hashedPath, dataFileName).String()
+ return joinLocationPath(p.dataPath, hashedPath, dataFileName)
} else {
- return p.simpleLocationProvider.dataPath.JoinPath(hashedPath + "-" + dataFileName).String()
+ return joinLocationPath(p.dataPath, hashedPath+"-"+dataFileName)
}
}
-func newObjectStoreLocationProvider(tableLoc *url.URL, tableProps iceberg.Properties) (*objectStoreLocationProvider, error) {
- slp, err := newSimpleLocationProvider(tableLoc, tableProps)
- if err != nil {
- return nil, err
- }
-
+func newObjectStoreLocationProvider(tableLoc string, tableProps iceberg.Properties) *objectStoreLocationProvider {
return &objectStoreLocationProvider{
- simpleLocationProvider: slp,
+ simpleLocationProvider: newSimpleLocationProvider(tableLoc, tableProps),
includePartitionPaths: tableProps.GetBool(WriteObjectStorePartitionedPathsKey,
WriteObjectStorePartitionedPathsDefault),
- }, nil
+ }
}
func LoadLocationProvider(tableLocation string, tableProps iceberg.Properties) (LocationProvider, error) {
- u, err := url.Parse(tableLocation)
- if err != nil {
- return nil, err
- }
-
if tableProps.GetBool(ObjectStoreEnabledKey, ObjectStoreEnabledDefault) {
- return newObjectStoreLocationProvider(u, tableProps)
+ return newObjectStoreLocationProvider(tableLocation, tableProps), nil
}
- return newSimpleLocationProvider(u, tableProps)
+ return newSimpleLocationProvider(tableLocation, tableProps), nil
}
diff --git a/table/locations_test.go b/table/locations_test.go
index c8f83824a..4c38b71c2 100644
--- a/table/locations_test.go
+++ b/table/locations_test.go
@@ -18,16 +18,120 @@
package table_test
import (
+ "os"
+ "path/filepath"
"strings"
"testing"
"github.com/apache/iceberg-go"
+ "github.com/apache/iceberg-go/io"
"github.com/apache/iceberg-go/table"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
+func TestLocationProviderLocalPaths(t *testing.T) {
+ for _, objectStore := range []string{"false", "true"} {
+ for _, partitionPaths := range []string{"false", "true"} {
+ for _, name := range []string{"my table", "literal%20table", "100% complete", "table#fragment", "table?query"} {
+ t.Run(objectStore+"/"+partitionPaths+"/"+name, func(t *testing.T) {
+ if strings.Contains(name, "?") && os.PathSeparator == '\\' {
+ t.Skip("question marks are not valid in Windows filenames")
+ }
+ root := filepath.ToSlash(filepath.Join(t.TempDir(), name))
+ for _, custom := range []bool{false, true} {
+ props := iceberg.Properties{
+ table.ObjectStoreEnabledKey: objectStore,
+ table.WriteObjectStorePartitionedPathsKey: partitionPaths,
+ }
+ dataDir, metadataDir := root+"/data", root+"/metadata"
+ if custom {
+ dataDir, metadataDir = root+"/custom data", root+"/custom metadata"
+ props[table.WriteDataPathKey] = dataDir
+ props[table.WriteMetadataPathKey] = metadataDir
+ }
+ provider, err := table.LoadLocationProvider(root, props)
+ require.NoError(t, err)
+ metadataFile, err := provider.NewTableMetadataFileLocation(1)
+ require.NoError(t, err)
+ for _, file := range []struct{ location, dir string }{
+ {provider.NewDataLocation("data.parquet"), dataDir},
+ {provider.NewDataLocation("partition=value/data.parquet"), dataDir},
+ {provider.NewMetadataLocation("manifest.avro"), metadataDir},
+ {metadataFile, metadataDir},
+ } {
+ require.True(t, strings.HasPrefix(file.location, file.dir+"/"), "%q is outside %q", file.location, file.dir)
+ require.NotContains(t, file.location, `\`)
+ writer, err := (io.LocalFS{}).Create(file.location)
+ require.NoError(t, err)
+ require.NoError(t, writer.Close())
+ _, err = os.Stat(file.location)
+ require.NoError(t, err)
+ }
+ }
+ })
+ }
+ }
+ }
+}
+
+func TestLocationProviderLiteralPaths(t *testing.T) {
+ for _, tt := range []struct{ location, data, metadata string }{
+ {"my table", "my table/data/a", "my table/metadata/manifest.avro"},
+ {"100% complete", "100% complete/data/a", "100% complete/metadata/manifest.avro"},
+ {"literal%20table", "literal%20table/data/a", "literal%20table/metadata/manifest.avro"},
+ {"table#fragment", "table#fragment/data/a", "table#fragment/metadata/manifest.avro"},
+ {"table?query", "table?query/data/a", "table?query/metadata/manifest.avro"},
+ {"db:2024", "db:2024/data/a", "db:2024/metadata/manifest.avro"},
+ {"my table:v2", "my table:v2/data/a", "my table:v2/metadata/manifest.avro"},
+ {"dir/table:part%20", "dir/table:part%20/data/a", "dir/table:part%20/metadata/manifest.avro"},
+ {"s3://bucket/my table", "s3://bucket/my table/data/a", "s3://bucket/my table/metadata/manifest.avro"},
+ {"s3://bucket/100% complete", "s3://bucket/100% complete/data/a", "s3://bucket/100% complete/metadata/manifest.avro"},
+ {"s3://bucket/literal%20#?", "s3://bucket/literal%20#?/data/a", "s3://bucket/literal%20#?/metadata/manifest.avro"},
+ {"gs://bucket/my table", "gs://bucket/my table/data/a", "gs://bucket/my table/metadata/manifest.avro"},
+ {"az://bucket/my table", "az://bucket/my table/data/a", "az://bucket/my table/metadata/manifest.avro"},
+ {"file:///tmp/my table", "file:///tmp/my table/data/a", "file:///tmp/my table/metadata/manifest.avro"},
+ {"s3://bucket/a//./b/../table///", "s3://bucket/a//./b/../table/data/a", "s3://bucket/a//./b/../table/metadata/manifest.avro"},
+ {"dir//./other/../table/", "dir//./other/../table/data/a", "dir//./other/../table/metadata/manifest.avro"},
+ {"/", "/data/a", "/metadata/manifest.avro"},
+ {"C:/my table", "C:/my table/data/a", "C:/my table/metadata/manifest.avro"},
+ {`C:\my table`, `C:\my table/data/a`, `C:\my table/metadata/manifest.avro`},
+ {`s3://bucket/literal\name`, `s3://bucket/literal\name/data/a`, `s3://bucket/literal\name/metadata/manifest.avro`},
+ } {
+ t.Run(tt.location, func(t *testing.T) {
+ provider, err := table.LoadLocationProvider(tt.location, nil)
+ require.NoError(t, err)
+ assert.Equal(t, tt.data, provider.NewDataLocation("a"))
+ assert.Equal(t, tt.metadata, provider.NewMetadataLocation("manifest.avro"))
+ })
+ }
+}
+
+func TestLocationProviderLiteralCustomPaths(t *testing.T) {
+ for _, tt := range []struct{ objectStore, partitionPaths, data string }{
+ {"false", "false", "s3://bucket/my data%20#?/a"},
+ {"false", "true", "s3://bucket/my data%20#?/a"},
+ {"true", "false", "s3://bucket/my data%20#?/0101/0110/1001/10110010-a"},
+ {"true", "true", "s3://bucket/my data%20#?/0101/0110/1001/10110010/a"},
+ } {
+ t.Run(tt.objectStore+"/"+tt.partitionPaths, func(t *testing.T) {
+ provider, err := table.LoadLocationProvider("s3://bucket/my table", iceberg.Properties{
+ table.ObjectStoreEnabledKey: tt.objectStore,
+ table.WriteObjectStorePartitionedPathsKey: tt.partitionPaths,
+ table.WriteDataPathKey: "s3://bucket/my data%20#?///",
+ table.WriteMetadataPathKey: "gs://bucket/100% metadata/./../here/",
+ })
+ require.NoError(t, err)
+ assert.Equal(t, tt.data, provider.NewDataLocation("a"))
+ assert.Equal(t, "s3://bucket/my data%20#?/partition=a b%20#?/file.parquet",
+ provider.NewDataLocation("partition=a b%20#?/file.parquet"))
+ assert.Equal(t, "gs://bucket/100% metadata/./../here/manifest #?%.avro",
+ provider.NewMetadataLocation("manifest #?%.avro"))
+ })
+ }
+}
+
func TestLocationProviderMetadataDefaultLocation(t *testing.T) {
provider, err := table.LoadLocationProvider("table_location", nil)
require.NoError(t, err)