JingsongLi commented on PR #659: URL: https://github.com/apache/paimon-rust/pull/659#issuecomment-5235447590
Thanks for adding the fixed-bucket writer. The low-level writer is mostly consistent with PyPaimon, but the high-level distributed workflow is not aligned yet. ### [P1] The integration does not select the fixed-bucket path PyPaimon's Ray sink automatically selects fixed-bucket writing when `postpone.batch-write-fixed-bucket=true`, gathers global statistics, creates one shared plan, and passes it to every worker ([source](https://github.com/apache/paimon/blob/aebc9846fe0d574b48cbb62ec3161a5d47dfca72/paimon-python/pypaimon/write/ray_datasink.py#L294-L336)). This PR only exposes an explicit Rust/C builder. The DataFusion integration is unchanged, and Rust does not define the corresponding boolean option. Therefore, the normal high-level Rust write path still uses postpone buckets rather than Python's fixed-bucket workflow. Could we either wire this into the high-level batch integration or explicitly scope the PR to low-level primitives rather than PyPaimon parity? ### [P1] Sharing the bucket-count plan is insufficient for safe distributed writes PyPaimon preclusters data by partition, bucket, and primary key before dispatching it to writers ([source](https://github.com/apache/paimon/blob/aebc9846fe0d574b48cbb62ec3161a5d47dfca72/paimon-python/pypaimon/write/ray_datasink.py#L366-L440)). This is important because independent writers targeting the same bucket can restore the same sequence-number state; PyPaimon explicitly documents direct distributed primary-key writes as unsafe without this routing ([source](https://github.com/apache/paimon/blob/aebc9846fe0d574b48cbb62ec3161a5d47dfca72/paimon-python/pypaimon/ray/shuffle.py#L80-L127)). The new C API shares only `partition -> total_buckets`. Each writer can still receive arbitrary rows and independently write the same `(partition, bucket)`. The distributed test uses distinct IDs and therefore does not exercise the conflicting-worker case. Please add an ownership/routing contract—ideally one writer per `(partition, bucket)`—and a regression test where two workers would otherwise write the same bucket or primary key. ### [P2] Rust exposes a resolved plan, but not the Python-equivalent planner PyPaimon exposes `PostponeBucketPlanner`, including current metadata, input partition statistics, row/size target precedence, and active postpone-row handling ([source](https://github.com/apache/paimon/blob/aebc9846fe0d574b48cbb62ec3161a5d47dfca72/paimon-python/pypaimon/write/postpone_bucket.py#L194-L347)). Rust exposes only `PostponeBucketPlan::from_arrow`, so every distributed caller must independently reimplement the planning algorithm and compatible size estimation. Exposing a planner/statistics API would make the shared-plan path usable without duplicating internal rules. One clarification: buffering in the low-level writer without a supplied plan is aligned with PyPaimon's direct writer. The missing parity is primarily the coordinator/integration layer that computes one global plan and safely partitions work across writers. Until these gaps are addressed, I would describe this PR as providing the core Rust/C fixed-bucket primitives, rather than full alignment with PyPaimon's distributed batch-write workflow. -- 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]
