Skip to content

[core] Route overwrite writes by partition bucket layout - #10170

Closed
dwangatt wants to merge 5 commits into
apache:masterfrom
atlassian-forks:dwang/partition-layout-overwrite
Closed

dwangatt wants to merge 5 commits into
apache:masterfrom
atlassian-forks:dwang/partition-layout-overwrite

Conversation

@dwangatt

Copy link
Copy Markdown
Contributor

Summary

This is a focused follow-up extracted from #9370, following the merged partition-layout scan prerequisite (#10052).

This PR makes partition bucket layout a core write contract:

  • adds PartitionBucketMapping to resolve the current bucket count for each partition;
  • adds write APIs that carry the routing bucket count:
    write(partition, bucket, totalBuckets, data);
  • propagates the mapping into write restore so empty buckets recover the partition's active layout;
  • ensures core overwrite writes can route and stamp files using the target partition layout;
  • rejects the legacy write(partition, bucket, data) path when
    bucket.per-partition-count-enabled=true on a partitioned table.

The legacy path only carries a bucket id and cannot establish which bucket count was used for hashing. Rejecting it prevents a caller from silently routing a row with the table-level bucket count while the target partition uses a different layout.

Scope

This PR intentionally does not expose complete engine support for the option:

  • Flink routing and end-to-end integration will be added in a follow-up PR.
  • Spark will reject writes to tables with per-partition bucket counts until it has a safe routing implementation.
  • Documentation and end-to-end tests will follow with the Flink integration.

Verification

  • mvn -q -pl paimon-core -DskipTests compile
  • mvn -q -pl paimon-core spotless:check
  • git diff --check

The focused restore test was updated to cover legacy-path rejection, stale-layout rejection, and a successful explicit partition-layout write.

.withDescription(
"Whether to ignore the order of the buckets when reading data from an append-only table.");

public static final ConfigOption<Boolean> BUCKET_PER_PARTITION_COUNT_ENABLED =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConfigOptionsDocsCompletenessITCase discovers this new CoreOptions entry, but bucket.per-partition-count-enabled is absent from the generated configuration docs. The focused test currently fails with: Option bucket.per-partition-count-enabled in class org.apache.paimon.CoreOptions is not documented. This also explains the failing Core and integrations jobs on both JDK 8 and JDK 11. Please regenerate the configuration documentation according to paimon-docs/README.md (or explicitly exclude the option if it is intentionally internal) so the required completeness check passes.

@JingsongLi

Copy link
Copy Markdown
Contributor

Closing this PR under the end-to-end production-value criterion for this review pass. The PR introduces a public bucket.per-partition-count-enabled option and changes the core write contract, but its own scope statement leaves Flink routing, Spark's safe-write handling, configuration documentation, and engine-level end-to-end tests to later PRs. As submitted, there is no supported engine write path that demonstrates a partition being rescaled and then written/read safely with the new layout. The legacy core write path is deliberately rejected when the option is enabled, so merging this prerequisite alone would expose an option that users cannot safely use end to end.

I checked the full 23-file diff and ran the focused core suite after packaging the codegen loader: PartitionBucketMappingTest, FixedBucketWriteSelectorTest, FixedBucketRowKeyExtractorTest, FileSystemWriteRestoreTest, and FileStoreCommitTest passed (97 tests). This verifies the core pieces but does not close the engine-level gap. The new option also is absent from generated config docs, as noted in the existing review. The latest JDK 8/11 Core CI jobs stop in S3FileIOTest because the MinIO image cannot be fetched, which is unrelated to this feature.

Please bring the core routing contract back together with at least one complete engine write/read path, explicit behavior for the other engines, generated option docs, and a rescale-then-write/read integration test. That would make the production behavior reviewable as a whole.

@JingsongLi JingsongLi closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants