JingsongLi commented on code in PR #1164:
URL: https://github.com/apache/paimon-rust/pull/1164#discussion_r4371214958
##########
crates/paimon/src/table/kv_file_reader.rs:
##########
@@ -711,6 +764,45 @@ impl KeyValueFileReader {
.map(|budget| Arc::new(budget.without_prefetch()))
};
let mut file_streams: Vec<ArrowRecordBatchStream> =
Vec::new();
+ // One sorted run never repeats a key, so under these
engines
+ // every key is a lone version and needs no merge. Retract
+ // rows are still dropped per batch below: older Rust
writers
+ // recorded `delete_row_count = 0` for files holding them.
+ let mut skip_merge = skip_single_runs
+ && input_stream_count == 1
+ && matches!(
+ config.merge_engine,
+ MergeEngine::Deduplicate | MergeEngine::FirstRow
+ );
+ // A file written under another engine may hold several
+ // versions of a key (the aggregation writer kept them
before
+ // #839), and ALTER can switch the engine afterwards.
+ if skip_merge {
+ 'files: for run in &merge_group {
+ for file in &run.files {
+ // Dynamic options can override the engine but
keep
+ // the schema ID, so only then check the
stored one.
+ if file.file.schema_id == table_schema_id
+ && !config.merge_engine_overridden
+ {
+ continue;
Review Comment:
[P2] Check the writer engine after historical schema resolution
The absence of a dynamic `merge-engine` key does not prove that this read
schema has the stored engine. After a supported persisted aggregation →
deduplicate ALTER, `copy_with_time_travel_strict({"scan.version":"1"})`
resolves the historical schema ID 0 but `historical_field_options` retains the
current deduplicate engine. The files also have ID 0 and this flag is false, so
this `continue` bypasses their stored aggregation schema. With real pre-#839
aggregation Parquet containing `(1,seq0,10),(1,seq1,11)` and a disjoint key-2
file, the ordinary two-file nonraw scan returns `[(1,10),(1,11),(2,20)]`; the
identical probe on exact base 4cf34379 returns `[(1,11),(2,20)]`. The
non-time-travel ALTER control passes on this head. Rewrapping a dynamically
overridden table with `copy_with_resolved_schema` clears the flag and
reproduces the same defect. Establish the persisted writer engine independently
of this dynamic-option bookkeeping (or conservatively retain merging for such
resolved
views), and cover the normal historical-read lifecycle.
--
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]
