Dem1241 commented on code in PR #2368:
URL: https://github.com/apache/iceberg-go/pull/2368#discussion_r4436778258
##########
table/rolling_data_writer.go:
##########
@@ -309,16 +295,10 @@ func (w *writerFactory) openFileWriter(ctx
context.Context, partitionPath string
fileName := dataFileName(*w.writeUUID, cnt, partitionID, fileCount,
strings.ToLower(string(w.fileFormat)))
- var filePath string
if partitionPath != "" {
- partitionLoc, err := w.partitionLocProvider(partitionPath)
- if err != nil {
- return nil, err
- }
- filePath = partitionLoc.NewDataLocation(fileName)
- } else {
- filePath = w.locProvider.NewDataLocation(fileName)
+ fileName = partitionPath + "/" + fileName
Review Comment:
went with a separate argument, it's `NewDataLocation(dataFileName,
partitionPath string)` now, same order as pyiceberg. `""` means unpartitioned.
the `path.Base` is gone and the title has the `!`.
##########
table/locations_test.go:
##########
@@ -125,7 +125,7 @@ func TestLocationProviderLiteralCustomPaths(t *testing.T) {
})
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",
+ assert.Equal(t, tt.partitioned,
Review Comment:
added `a=1/b=2/file.parquet` for both values. the `false` row equals the
single-level one, and the `true` row pins the hash over the full name.
--
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]
