You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "Aitozi (via GitHub)" <gi...@apache.org> on 2024/04/29 15:14:43 UTC
[PR] [core] Fix the merge order in lookup compaction [paimon]
Aitozi opened a new pull request, #3286:
URL: https://github.com/apache/paimon/pull/3286
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
```java
mergeFunction2.reset();
if (highLevel != null) {
mergeFunction2.add(highLevel);
}
candidates.forEach(mergeFunction2::add);
KeyValue result = mergeFunction2.getResult();
```
When we find a high-level record, we now add it to the merge function first. We previously assumed that the high-level record had a smaller sequence number, which is incorrect. To obtain accurate results, we should add the high-level record and candidates in order.
<!-- Linking this pull request to the issue -->
<!-- What is the purpose of the change -->
### Tests
<!-- List UT and IT cases to verify this change -->
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Fix the merge order in lookup compaction [paimon]
Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on code in PR #3286:
URL: https://github.com/apache/paimon/pull/3286#discussion_r1584026769
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupChangelogMergeFunctionWrapper.java:
##########
@@ -136,12 +143,7 @@ public ChangelogResult getResult() {
}
Review Comment:
See this [test](https://github.com/apache/paimon/pull/3286/files#diff-11928292ecbbd8b574d1a2329427e15330667b67d5bb979a151cf4f0843959f1R334). We have higher sequence in the high level value, the L0's seq is lower, if we add the high level value first, the result is wrong
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Fix the merge order in lookup compaction [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #3286:
URL: https://github.com/apache/paimon/pull/3286#discussion_r1583898373
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupChangelogMergeFunctionWrapper.java:
##########
@@ -136,12 +143,7 @@ public ChangelogResult getResult() {
}
Review Comment:
actually `mergeFunction.candidates()` is the right order? maybe we can use it.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Fix the merge order in lookup compaction [paimon]
Posted by "Aitozi (via GitHub)" <gi...@apache.org>.
Aitozi commented on code in PR #3286:
URL: https://github.com/apache/paimon/pull/3286#discussion_r1584025160
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupChangelogMergeFunctionWrapper.java:
##########
@@ -136,12 +143,7 @@ public ChangelogResult getResult() {
}
Review Comment:
The `mergeFunction.candidates()` are in the correct order, but we need to determine the right position when a new high-level record is found. Currently, it has not been compared using the user-defined sequence field or sequence number.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [core] Fix the merge order in lookup compaction [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #3286:
URL: https://github.com/apache/paimon/pull/3286
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org