JingsongLi commented on PR #659:
URL: https://github.com/apache/paimon-rust/pull/659#issuecomment-5250998259
After re-reading the latest implementation, I think the remaining issue is
the **top-level API boundary**, rather than the internal organization of the
Rust writer.
The Rust API is now properly separated:
```text
Table::new_write_builder()
-> WriteBuilder
-> TableWrite / TableCommit
Table::new_postpone_fixed_bucket_write_builder()
-> PostponeFixedBucketWriteBuilder
-> PostponeFixedBucketTableWrite / PostponeFixedBucketTableCommit
```
However, the C API only has a dedicated constructor. Immediately after
construction, the postpone path is merged back into the standard opaque handles
and standard operations:
```text
paimon_table_new_postpone_fixed_bucket_write_builder()
-> paimon_write_builder
-> paimon_write_builder_new_write()
-> paimon_table_write
-> paimon_table_write_prepare_commit()
-> paimon_commit_messages
-> paimon_table_commit_commit()
```
That shared API is why the binding now needs `WriteBuilderKind`,
`TableWriteKind`, `TableCommitKind`, and `WriteKind` / `WriteContext`, with
runtime dispatch repeated at every lifecycle stage. In other words, these enums
are compensating for erasing the type separation that the Rust API already
provides.
My expectation is that postpone fixed-bucket writing remains isolated from
the first public API boundary through commit, using dedicated opaque C types
and functions, for example:
```text
paimon_postpone_fixed_bucket_write_builder
-> paimon_postpone_fixed_bucket_table_write
-> paimon_postpone_fixed_bucket_commit_messages
-> paimon_postpone_fixed_bucket_table_commit
```
The standard `paimon_write_builder`, `paimon_table_write`, and
`paimon_table_commit` APIs should not need to know about postpone fixed-bucket
writing. Arrow import, schema validation, handle allocation, and other
mechanics can still be shared through private helpers, so this separation
should not require duplicating the implementation.
With that boundary, the C-side `WriteBuilderKind` should no longer be
necessary, and the other lifecycle-wide kind dispatches can likely be removed
as well. This would also make invalid standard/postpone combinations
unrepresentable at the API level instead of detecting them later through
`WriteContext`.
I consider this an architectural issue rather than a naming nit: the
dedicated Rust abstraction should be preserved by the C API instead of being
flattened and reconstructed through runtime tags.
--
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]
