lxy-9602 commented on code in PR #224:
URL: https://github.com/apache/paimon-cpp/pull/224#discussion_r3834929670
##########
include/paimon/realtime/realtime_store.h:
##########
@@ -40,6 +42,29 @@ namespace paimon {
class MemoryPool;
class Predicate;
+struct PAIMON_EXPORT AppendRealtimeStoreCreateConfig {};
+
+struct PAIMON_EXPORT PrimaryKeyRealtimeStoreCreateConfig {
+ std::vector<std::string> primary_keys;
+ /// Largest sequence restored from the committed snapshot. A PK store
assigns one contiguous
+ /// sequence to every mutation in `Write` order, starting at the next
value, and rejects
+ /// `Write` before the assigned sequence would exceed `INT64_MAX - 1`.
+ int64_t restore_max_sequence_number;
+};
+
+using RealtimeStoreCreateConfig =
+ std::variant<AppendRealtimeStoreCreateConfig,
PrimaryKeyRealtimeStoreCreateConfig>;
+
+struct PAIMON_EXPORT RealtimeStoreCreateRequest {
+ /// Complete table write schema whose ownership is transferred to the
factory.
+ std::unique_ptr<::ArrowSchema> write_schema;
+ std::map<std::string, std::string> options;
+ std::shared_ptr<MemoryPool> memory_pool;
+ std::map<std::string, std::string> partition;
+ int32_t bucket = -1;
+ RealtimeStoreCreateConfig mode_config;
+};
+
Review Comment:
Thank you for the contribution. The code looks clear and well organized.
Before diving into the detailed review, I would like to discuss two design
points.
First, it seems that internal sequence-number assignment and per-batch
primary-key sorting are currently handled inside the realtime store
implementation. I suggest moving these responsibilities into the Paimon
framework instead.
The framework could assign offsets and sequence numbers, append internal
fields such as `_VALUE_KIND`, `_SEQUENCE_NUMBER`, and `_REALTIME_OFFSET`, and
physically sort each input Arrow batch before passing it to the store plugin.
The plugin would then only manage storage concerns, without needing to
understand PK sorting rules, sequence fields, or merge-engine semantics.
Query and prepare-commit could convert these already sorted batches into
`KeyValueRecordReader`s and reuse the existing `SortMergeReader` and merge
functions. The flush path could also accept sorted readers directly, avoiding
sequence reassignment and repeated per-batch sorting. This would make custom
plugins easier to implement and allow realtime reads and writes to reuse the
frameworkâs existing merge-engine and sequence-field behavior.
I think this can be the first-stage solution. If profiling later shows that
copying data to produce physically sorted Arrow batches is a real write-path
bottleneck, we could introduce a shallow-copy mode based on sorted indices.
That would require significantly more interface changes, so I suggest
optimizing it only after it becomes an observed hotspot.
Second, the in-memory store could keep PK statistics for each batch, such as
min/max values. Predicates on value fields may not be pushable, but
`predicate_for_keys` should be applicable to these statistics so irrelevant
in-memory batches can be pruned during reads. This optimization could also be
implemented in a follow-up PR.
--
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]
