zjw1111 commented on code in PR #199:
URL: https://github.com/apache/paimon-cpp/pull/199#discussion_r3800501787
##########
include/paimon/realtime/realtime_context.h:
##########
@@ -66,29 +65,12 @@ struct PAIMON_EXPORT RealtimePartitionBucket {
/// Largest committed offset for each partition-bucket.
using RealtimeOffsetMap = std::map<RealtimePartitionBucket, int64_t>;
-/// Memory indexer and its initial offset resolved from committed and retained
memory progress.
-struct PAIMON_EXPORT RealtimeMemIndexerState {
- /// Plugin instance associated with the requested partition-bucket.
- std::shared_ptr<MemIndexer> indexer;
- /// First offset after both committed rows and rows currently retained by
the indexer.
- int64_t initial_offset;
-};
-
-/// One partition-bucket and the immutable plugin view captured for a table
scan.
-struct PAIMON_EXPORT RealtimePartitionBucketView {
- /// Partition-bucket associated with this view.
- RealtimePartitionBucket partition_bucket;
- /// Plugin instance that creates readers from `read_view`.
- std::shared_ptr<MemIndexer> indexer;
- /// Immutable rows pinned for one query plan.
- std::shared_ptr<MemReadView> read_view;
-};
-
/// Shared context that owns the `MemIndexer` instances used by a real-time
writer.
///
-/// Applications share one context between `WriteContext` and `ScanContext`.
The context uses
-/// either the default Arrow implementation or an application-provided factory
and keeps each
-/// created indexer available across writes, prepare-commit operations, and
process-local reads.
+/// Applications share one context between `WriteContext`, `ScanContext`, and
`ReadContext`. The
+/// context uses either the default Arrow implementation or an
application-provided factory and
+/// keeps each created indexer available across writes, prepare-commit
operations, and
+/// process-local reads.
class PAIMON_EXPORT RealtimeContext {
Review Comment:
Thanks for the detailed analysis. You are right that `RealtimeContext`
currently only transports the concrete `RealtimeContextImpl` and does not
define a behavioral contract of its own.
That said, this is a general design question in our codebase rather than
something introduced by this PR. `Split`, `Predicate` and several other public
types follow the same shape: an abstract public base plus a downcast to the
concrete implementation at the call sites. Reworking the hierarchy only for
`RealtimeContext` here would leave the codebase inconsistent and would grow the
scope of this PR considerably.
So we would prefer not to address it in this PR. We will discuss this
hierarchy design as a whole and then apply a consistent change across all of
these types in follow-up PRs. Would that work for you?
--
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]
