You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "JingsongLi (via GitHub)" <gi...@apache.org> on 2024/03/11 04:36:34 UTC
[PR] [core] Refactor ChangelogMergeTreeRewriter to lookup free [incubator-paimon]
JingsongLi opened a new pull request, #2986:
URL: https://github.com/apache/incubator-paimon/pull/2986
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
<!-- Linking this pull request to the issue -->
`ChangelogMergeTreeRewriter` should get rid of lookup concept.
<!-- 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] Refactor ChangelogMergeTreeRewriter to lookup free [incubator-paimon]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #2986:
URL: https://github.com/apache/incubator-paimon/pull/2986#discussion_r1519155037
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/ChangelogMergeTreeRewriter.java:
##########
@@ -173,24 +174,19 @@ private CompactResult rewriteChangelogCompaction(
List<DataFileMeta> before = extractFilesFromSections(sections);
List<DataFileMeta> after =
- rewriteCompactFile
+ compactFileWriter != null
? compactFileWriter.result()
: before.stream()
.map(x -> x.upgrade(outputLevel))
.collect(Collectors.toList());
- if (deletionVectorsMaintainer != null) {
Review Comment:
Looks like here is forgot to add to notifyCompactBefore(before)
--
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] Refactor ChangelogMergeTreeRewriter to lookup free [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2986:
URL: https://github.com/apache/incubator-paimon/pull/2986
--
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] Refactor ChangelogMergeTreeRewriter to lookup free [incubator-paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2986:
URL: https://github.com/apache/incubator-paimon/pull/2986#discussion_r1519169007
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupMergeTreeCompactRewriter.java:
##########
@@ -75,17 +75,18 @@ public LookupMergeTreeCompactRewriter(
userDefinedSeqComparator,
mfFactory,
mergeSorter,
- lookupStrategy,
- deletionVectorsMaintainer);
- if (lookupStrategy.deletionVector) {
- checkArgument(
- deletionVectorsMaintainer != null,
- "deletionVectorsMaintainer should not be null, there is a bug.");
- }
+ produceChangelog,
+ dvMaintainer != null);
+ this.dvMaintainer = dvMaintainer;
this.lookupLevels = lookupLevels;
this.wrapperFactory = wrapperFactory;
}
+ @Override
+ protected void notifyCompactBefore(List<DataFileMeta> files) {
Review Comment:
Here should notify `DeletionVectorsMaintainer`.
--
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] Refactor ChangelogMergeTreeRewriter to lookup free [incubator-paimon]
Posted by "Zouxxyy (via GitHub)" <gi...@apache.org>.
Zouxxyy commented on code in PR #2986:
URL: https://github.com/apache/incubator-paimon/pull/2986#discussion_r1519163047
##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupMergeTreeCompactRewriter.java:
##########
@@ -75,17 +75,18 @@ public LookupMergeTreeCompactRewriter(
userDefinedSeqComparator,
mfFactory,
mergeSorter,
- lookupStrategy,
- deletionVectorsMaintainer);
- if (lookupStrategy.deletionVector) {
- checkArgument(
- deletionVectorsMaintainer != null,
- "deletionVectorsMaintainer should not be null, there is a bug.");
- }
+ produceChangelog,
+ dvMaintainer != null);
+ this.dvMaintainer = dvMaintainer;
this.lookupLevels = lookupLevels;
this.wrapperFactory = wrapperFactory;
}
+ @Override
+ protected void notifyCompactBefore(List<DataFileMeta> files) {
Review Comment:
this override can remove
--
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