JingsongLi commented on code in PR #9473:
URL: https://github.com/apache/paimon/pull/9473#discussion_r3889619057
##########
paimon-core/src/main/java/org/apache/paimon/utils/DataEvolutionUtils.java:
##########
@@ -201,4 +202,105 @@ public static Range
checkContiguousRowRange(List<DataFileMeta> files) {
merged);
return merged.get(0);
}
+
+ /**
+ * Groups data-evolution files by normal-file row ranges.
+ *
+ * <p>Blob and vector files may span several adjacent normal files. Such
sidecar files are
+ * included in every anchor group they intersect, so each group can select
just its normal-file
+ * range without duplicating the physical sidecar.
+ */
+ public static <T> List<List<T>> groupByNormalFileRange(
Review Comment:
[Suggestion] Model an anchored range explicitly instead of returning nested
lists. The important contract here is that a sidecar may be attached to several
anchor groups as the same object identity, while an unanchored group has
different semantics. That contract is currently erased by `List<List<T>>`, so
scan, split generation, `DataSplit`, and split read each have to rediscover
anchors, shared-sidecar occurrences, and identity deduplication. A small
`NormalRangeGroup<T>` (anchor range, normal entries, intersecting sidecars, and
unanchored state) would put coverage ownership in one place and make the
downstream code substantially easier to audit; a flattening adapter could keep
callers incremental.
##########
paimon-common/src/main/java/org/apache/paimon/format/FormatReaderFactory.java:
##########
@@ -60,5 +61,11 @@ interface Context {
default ReadBatchSizer readBatchSizer() {
return null;
}
+
+ /** Metadata shared by readers of the same physical file within one
split. */
+ @Nullable
+ default Map<Path, Object> metadataCache() {
Review Comment:
[Suggestion] Hide this untyped map behind a metadata-cache abstraction.
Adding `Map<Path, Object>` to the general format SPI makes cache lifetime and
value type an implicit cross-module contract, and BLOB/video readers now
duplicate unchecked get/cast/load/put logic. A typed `getOrLoad(Path, Class<T>,
loader)` (or format-scoped cache key) owned by `FormatReaderContext` would
centralize miss/error behavior, prevent accidental type collisions, and let
`DataEvolutionSplitRead` pass a small per-split read session instead of
plumbing a mutable raw map through its reader-building methods.
##########
paimon-core/src/main/java/org/apache/paimon/table/source/DataEvolutionSplitGenerator.java:
##########
@@ -79,6 +102,111 @@ public List<SplitGroup> splitForBatch(List<DataFileMeta>
input) {
.collect(Collectors.toList());
}
+ private List<List<List<DataFileMeta>>> packWithUniqueSidecars(
Review Comment:
[Suggestion] Encapsulate the packing state and make the weight calculation
side-effect free. `current`, `currentWeight`, `seenSidecars`, and
`fixedComponent` must move together, but the full reset is hand-written in two
branches and `initialRangeWeight` mutates `fixedComponent` as an output
parameter. A private `PackingState` with one finish/reset transition plus an
`InitialWeight { weight, fixedSidecars }` result would make the coverage
invariant visible and reduce the chance that the Java and Python mirror
implementations drift during the next edge-case change.
--
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]
