You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "YannByron (via GitHub)" <gi...@apache.org> on 2024/04/10 10:17:08 UTC
[PR] [core] support PartitionKeyVisitor [paimon]
YannByron opened a new pull request, #3193:
URL: https://github.com/apache/paimon/pull/3193
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #xxx
<!-- 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] support PartitionKeyVisitor [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #3193:
URL: https://github.com/apache/paimon/pull/3193#discussion_r1565173984
##########
paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/commands/DeleteFromPaimonTableCommand.scala:
##########
@@ -62,25 +65,26 @@ case class DeleteFromPaimonTableCommand(
table.partitionKeys().asScala,
sparkSession.sessionState.conf.resolver)
- // TODO: provide another partition visitor to support more partition predicate.
- val visitor = new OnlyPartitionKeyEqualVisitor(table.partitionKeys)
- val partitionPredicate = if (partitionCondition.isEmpty) {
- None
+ val visitor =
+ new PartitionKeyVisitor(table.partitionKeys, table.newReadBuilder.newScan.listPartitions)
+
+ val (partitionPredicate, candidatePartitions) = if (partitionCondition.isEmpty) {
+ (None, List.empty[BinaryRow])
} else {
- convertConditionToPaimonPredicate(
+ val predicate = convertConditionToPaimonPredicate(
partitionCondition.reduce(And),
relation.output,
rowType,
ignoreFailure = true)
+ (predicate, predicate.map(_.visit(visitor).asScala.toList).getOrElse(List.empty[BinaryRow]))
}
- // We do not have to scan table if the following three requirements are met:
- // 1) no other predicate;
- // 2) partition condition can convert to paimon predicate;
- // 3) partition predicate can be visit by OnlyPartitionKeyEqualVisitor.
+ // We have to scan table if the following three requirements are met:
+ // 1) non-partitioned table;
+ // 2) no partition predicate;
+ // 3) all the data in candidate partitions should be deleted;
Review Comment:
"all the data in candidate partitions should be deleted" => scan table?
##########
paimon-common/src/main/java/org/apache/paimon/predicate/PartitionKeyVisitor.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.predicate;
+
+import org.apache.paimon.annotation.VisibleForTesting;
+import org.apache.paimon.data.BinaryRow;
+import org.apache.paimon.types.DataType;
+import org.apache.paimon.utils.InternalRowUtils;
+
+import org.apache.paimon.shade.guava30.com.google.common.collect.Sets;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/** PartitionKeyVisitor. */
+public class PartitionKeyVisitor implements FunctionVisitor<Set<BinaryRow>> {
Review Comment:
What is `PartitionKeyVisitor`? How to use it? Can you add documentation here?
--
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] support PartitionKeyVisitor [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #3193:
URL: https://github.com/apache/paimon/pull/3193#issuecomment-2083879741
Alread fixed in https://github.com/apache/paimon/pull/3263 ? Close this now.
--
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] support PartitionKeyVisitor [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #3193:
URL: https://github.com/apache/paimon/pull/3193#issuecomment-2056337744
If your goal is to optimize the `DELETE` to the `dropPartitions`, we can determine if there are only predicates for partition fields. You can use `PartitionPredicteVisitor`, and then list Partitions, one by one, to test the predicate.
In this way, you don't need to write a `FunctionVisitor` class.
--
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] support PartitionKeyVisitor [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #3193:
URL: https://github.com/apache/paimon/pull/3193#issuecomment-2051429345
+1
--
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] support PartitionKeyVisitor [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi closed pull request #3193: [core] support PartitionKeyVisitor
URL: https://github.com/apache/paimon/pull/3193
--
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