JingsongLi commented on PR #10170:
URL: https://github.com/apache/paimon/pull/10170#issuecomment-5831687516

   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.
   


-- 
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]

Reply via email to