andygrove opened a new issue, #6956:
URL: https://github.com/apache/datafusion-comet/issues/6956
### What is the problem the feature request solves?
A table with `write.object-storage.enabled=true` falls back to
iceberg-java's writer. The eligibility gate declines it twice: the
`write.object-storage.enabled` property rule, and
`requireDefaultLocationProvider`, which admits only Iceberg's
`DefaultLocationProvider`. The object-storage layout is common on S3, where it
spreads data files across key prefixes to avoid request throttling, so it is
one of the fallbacks most likely to hit production tables (#5643).
### Describe the potential solution
Add iceberg-java's `ObjectStoreLocationProvider` layout to the native
writer, matched to the Iceberg version Comet runs against
(`LocationProviders.java` at each tag):
- **Iceberg 1.7 and newer** (the 1.8.1, 1.10.0 and 1.11.0 profiles): the
location is `<storage location>/<entropy>/<partition path>/<file name>`. The
entropy is the last 20 bits of `murmur3_32` over `<partition path>/<file name>`
(or the file name alone for an unpartitioned write), written in binary as
`xxxx/xxxx/xxxx/xxxxxxxx`. When the storage location is outside the table
location, the table location's `<parent>/<name>` goes after the entropy. With
`write.object-storage.partitioned-paths=false`, the partition path is left out
of both the hash input and the location, and, when no `<parent>/<name>` is
added, the last entropy directory is joined to the file name with `-`.
- **Iceberg 1.5.2** (the Spark 3.4 profile): one directory holding the
unpadded base64url encoding of the 4-byte murmur3 hash, and no
`partitioned-paths` property.
iceberg-rust's `ObjectStorageLocationGenerator` implements the 1.7+
algorithm at the pinned revision, but it builds the partition path with
iceberg-rust's own `partition_to_path`, which Comet already replaces with
`CometLocationGenerator` (`iceberg_partition_path.rs`) because it diverges from
iceberg-java. So this is an object-storage mode in `CometLocationGenerator`,
reusing iceberg-rust's hashing where it matches.
On the JVM side, read the storage location, context and
`includePartitionPaths` from the table's own `ObjectStoreLocationProvider`
rather than re-deriving them from properties. That keeps iceberg-java's
precedence between `write.data.path`, `write.object-storage.path` and
`write.folder-storage.path`, and its rejection of the two deprecated properties
from 1.10 on. `IcebergReflection.getDataLocation` calls `newDataLocation("")`,
which for this provider would put the hash of an empty name into the location,
so the object-storage case needs its own path.
Tests: write the same rows through both writers to a partitioned and an
unpartitioned table, with `partitioned-paths` true and false, and with a
`write.data.path` outside the table location, and compare the committed file
paths' layout. File names differ between the two writers (an accepted
divergence), so the entropy differs as well; compare the structure, and check
each native path's entropy against the murmur3 hash of its own name.
### Additional context
Part of #5643 and the native write epic #5649. A custom location provider
(`write.location-provider.impl`, or one that a custom `TableOperations`
supplies) stays a fallback: the native writer cannot call user code in the JVM.
--
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]
