[core][rest][spark] Add explicit Format Table partition locations - #9540
[core][rest][spark] Add explicit Format Table partition locations#9540sundapeng wants to merge 12 commits into
Conversation
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED
Implementation: FINDINGS
| requirePartitionManager().createPartitions(specs.asJava, ignoreIfExists) | ||
| } else { | ||
| requirePartitionManager() | ||
| .createPartitions(specs.asJava, ignoreIfExists, null, false, locations.asJava) |
There was a problem hiding this comment.
[P1] Validate the prospective registry atomically before registering locations.
This path validates each explicit location only against the table root and the default directory of that same spec, then immediately mutates the catalog. A batch can therefore assign the same or nested LOCATION values to different specs; MSCK can similarly add a default spec whose directory is already owned by another explicit spec. Registration succeeds, but scans, writes, and DROP later reject the full registry as overlapping, so normal SQL cannot even unregister the bad entry. Please make the catalog/REST mutation atomically validate the complete post-update ownership set for both ordinary create and /with-locations; a client-side listing alone would still race concurrent ADDs.
| // An absolute path without a scheme and file:/ name the same local filesystem. | ||
| scheme = scheme == null ? "file" : scheme.toLowerCase(Locale.ROOT); | ||
| String authority = uri.getAuthority(); | ||
| authority = authority == null ? "" : authority.toLowerCase(Locale.ROOT); |
There was a problem hiding this comment.
[P1] Canonicalize the filesystem identity before comparing ownership.
The ownership key uses the raw lower-cased scheme and authority, although different URI spellings can resolve to the same filesystem. For example, Hadoop can resolve hdfs://nn and hdfs://nn:8020 to the same NameNode, but this code treats them as separate roots. A table at hdfs://nn:8020/warehouse/t can therefore accept an explicit location such as hdfs://nn/warehouse/t, bypassing the table-root check and potentially reading the whole table as one partition or writing through a default sibling into explicitly owned data. Please derive the key from the resolved/canonical FileSystem URI, including default ports and configured aliases, and use that identity consistently for overlap checks and FileIO routing.
| String uriPath = uri.getPath(); | ||
| if (scheme == null | ||
| || scheme.isEmpty() | ||
| || uri.getUserInfo() != null |
There was a problem hiding this comment.
[P1] Do not reject URI forms used by supported filesystems.
These checks reject hdfs:///path because it has no authority, even though Paimon documents that form and HadoopFileIO can resolve it through the catalog Hadoop configuration. They also reject the standard ABFS form abfs://filesystem@account.dfs.core.windows.net/path, because java.net.URI parses filesystem as user info even though it is the Azure filesystem/container name rather than a credential. As a result, explicit partition locations are unusable for common HDFS deployments and standard ADLS Gen2 addresses. Please make validation scheme/FileIO-aware: qualify authorityless HDFS through CatalogContext, recognize the structural ABFS authority, and continue rejecting actual embedded credentials.
| if (RESTApi.containsCapability( | ||
| api.options().get(RESTCatalogInternalOptions.SERVER_CAPABILITIES), | ||
| RESTApi.FORMAT_TABLE_PARTITION_LOCATION_CAPABILITY) | ||
| && (count == null || count < 0)) { |
There was a problem hiding this comment.
[P1] Preserve the documented unknown-count fallback.
The OpenAPI description explicitly allows explicitPartitionLocationCount to be omitted, and Catalog#getExplicitPartitionLocationCount defines an empty result as a signal to load and validate the full registry. Requiring the count whenever the server advertises the location capability contradicts both contracts: a location-capable provider that cannot supply this optional optimization will make every getTable and table-detail conversion fail before any fallback can run. Please accept null as OptionalLong.empty() and use the existing full-registry path, or advertise count support through a separate capability that makes the field mandatory.
| override def run(sparkSession: SparkSession): Seq[Row] = { | ||
| val prefix = leadingPrefix(sparkSession) | ||
| val partitions = v2Table.partitionManager | ||
| val registeredPartitions = v2Table.partitionManager |
There was a problem hiding this comment.
[P2] Validate the full registry before scoped ANALYZE.
This query is already prefix-pruned, so checking location only on the returned partitions does not validate global path ownership or the authoritative explicit-location count. If selected default partition A has a directory that is also owned by an unselected explicit partition B, this guard passes and FormatTablePartitionStatsCollector reads that directory as A, probing explicitly owned data and replacing A statistics with B data. That state is reachable through the current ADD/MSCK registration gap or an inconsistent provider. Please run the same count-aware full-registry preflight used by scans and commits, then apply the ANALYZE prefix and reject selected explicit partitions.
Purpose
As a follow-up to #8750 and #8751, this adds Spark's standard explicit-location
form for registering existing data in a catalog-managed Format Table:
For
ADD PARTITION ... LOCATION, Spark canonicalizes and validates the absoluteURI before registration. It rejects roots, table ancestors, and the partition's
derived default directory, registers the location as partition metadata without
creating or probing the supplied directory, and verifies that the REST server
stored and echoed the same canonical value. Partitions added without
LOCATIONkeep their current derived-path behavior.
When explicit locations may exist, reads and mutation preflights validate the
complete catalog registry as one path-ownership set. Duplicate, ancestor,
descendant, table-root, and derived-path conflicts fail closed, as does a mismatch
between the authoritative explicit-location count and the listed registry.
Locations outside the table root use a
FileIOresolved from the client'sCatalogContext; locations under the table root continue using the table'sFileIO.Data in an explicitly located partition is read-only to Paimon. Write commits,
overwrites, truncates, and
ANALYZE TABLEoperations whose scope includes such apartition fail before publishing or deleting files or mutating catalog metadata.
DROP PARTITIONonly unregisters it and never probes or deletes the explicit URI.MSCK REPAIR TABLEleaves the registration intact and does not inspect or collectstatistics from that location.
The REST contract uses a dedicated
partitions/with-locationsresource and theformat-table-partition-location-v1capability token. A location-aware POST isrefused unless the server advertises the capability. Spark REST requests advertise
the client capability; Flink requests intentionally do not. The existing partition
resource and location-free payloads remain compatible.
This change is limited to catalog-managed Format Tables; it does not add
multi-location storage to native Paimon tables or change the table-level
SET LOCATIONrejection from #9435.Tests
paimon-apiandpaimon-coresuites covering JSON andREST compatibility, catalog delegation and caching, partition management, scans,
reads, commits, statistics,
FileIOrouting, registry consistency, and path ownership.FormatTablePartitionRepairTesttests, and2 REST capability-header tests.
clean package.