JingsongLi commented on code in PR #263:
URL: https://github.com/apache/paimon-rust/pull/263#discussion_r3109535485
##########
crates/paimon/src/table/sort_merge.rs:
##########
@@ -52,15 +77,56 @@ pub(crate) struct MergeRow {
pub user_sequences: Vec<Option<i128>>,
}
+impl MergeRow {
+ #[allow(dead_code)]
+ fn source_batch<'a>(
Review Comment:
If it's test-only infrastructure, consider moving it to #[cfg(test)] or
removing the allow and using it in production code. Dead code annotations tend
to accumulate.
##########
crates/paimon/src/table/table_write.rs:
##########
@@ -123,6 +124,19 @@ impl TableWrite {
let total_buckets = core_options.bucket();
let has_primary_keys = !schema.primary_keys().is_empty();
+ let table_name = table.identifier().full_name();
+ let partial_update_mode = PartialUpdateConfig::new(schema.options())
Review Comment:
This validation should happened in `KeyValueFileWriter`.
And should new merge_function, validation should happened in new.
##########
crates/paimon/src/table/table_read.rs:
##########
@@ -73,18 +73,42 @@ impl<'a> TableRead<'a> {
/// Returns an [`ArrowRecordBatchStream`].
pub fn to_arrow(&self, data_splits: &[DataSplit]) ->
crate::Result<ArrowRecordBatchStream> {
let has_primary_keys = !self.table.schema.primary_keys().is_empty();
+ let table_name = self.table.identifier().full_name();
+ PartialUpdateConfig::new(self.table.schema().options())
+ .validate_runtime_mode(has_primary_keys, &table_name)?;
let core_options = CoreOptions::new(self.table.schema.options());
+ let merge_engine = core_options.merge_engine()?;
+ let deletion_vectors_enabled = core_options.deletion_vectors_enabled();
+
+ if has_primary_keys
+ && merge_engine == MergeEngine::PartialUpdate
+ && deletion_vectors_enabled
+ {
+ return Err(crate::Error::Unsupported {
+ message: format!(
+ "Table '{table_name}' uses merge-engine=partial-update
with deletion-vectors.enabled=true, which is not supported yet"
+ ),
+ });
+ }
// PK table with Deduplicate engine: splits containing level-0 files
// need KeyValueFileReader for sort-merge dedup; splits with only
// compacted files (level > 0) can use the faster DataFileReader.
// FirstRow engine falls through — scan already skips level-0.
- if has_primary_keys
- && core_options
- .merge_engine()
- .is_ok_and(|e| e == crate::spec::MergeEngine::Deduplicate)
- {
- return self.read_pk_deduplicate(data_splits, &core_options);
+ if has_primary_keys {
+ return match merge_engine {
+ MergeEngine::Deduplicate =>
self.read_pk_deduplicate(data_splits, &core_options),
+ MergeEngine::PartialUpdate => {
+ self.read_pk_partial_update(data_splits, &core_options)
+ }
+ MergeEngine::FirstRow => {
Review Comment:
Do not modify first row. You can just judge `Deduplicate` and
`PartialUpdate` to `read_pk`.
##########
crates/paimon/src/table/table_scan.rs:
##########
@@ -247,8 +247,25 @@ fn merge_manifest_entries(entries: Vec<ManifestEntry>) ->
Vec<ManifestEntry> {
pub(super) fn can_push_down_limit_hint_for_scan(
data_predicates: &[Predicate],
row_ranges: Option<&[RowRange]>,
+ partial_update_enabled: bool,
Review Comment:
I cannot understand why here need to limit partial_update_enabled.
--
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]
