From dac3b0118c0ce1416a00a287ca46560c30e4432e Mon Sep 17 00:00:00 2001 From: Natasha Sehgal Date: Fri, 27 Jun 2025 09:15:07 -0700 Subject: [PATCH] Refactor MetadataDeleteNode to spi Plan (#25430) Summary: ## Description Refactor MetadeleteNode from com.facebook.presto.sql.planner.plan.MetadataDeleteNode to com.facebook.presto.spi.plan.MetadataDeleteNode, ## Motivation and Context Refactor so Metadata Optimization can be used in FB Metalake optimization ## Impact No impact Test Plan: - [x] Unit tests ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [x] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == NO RELEASE NOTE == ``` Rollback Plan: Differential Revision: D77249220 Pulled By: natashasehgal --- .../presto/sql/planner/BasePlanFragmenter.java | 2 +- .../presto/sql/planner/LocalExecutionPlanner.java | 2 +- .../presto/sql/planner/PlanFragmenterUtils.java | 2 +- .../presto/sql/planner/SplitSourceFactory.java | 2 +- .../optimizations/MetadataDeleteOptimizer.java | 2 +- .../sql/planner/plan/InternalPlanVisitor.java | 5 ----- .../sql/planner/planPrinter/PlanPrinter.java | 2 +- .../sanity/ValidateDependenciesChecker.java | 2 +- .../com/facebook/presto/util/GraphvizPrinter.java | 2 +- .../presto/spi}/plan/MetadataDeleteNode.java | 14 ++++++-------- .../com/facebook/presto/spi/plan/PlanVisitor.java | 5 +++++ 11 files changed, 19 insertions(+), 21 deletions(-) rename {presto-main-base/src/main/java/com/facebook/presto/sql/planner => presto-spi/src/main/java/com/facebook/presto/spi}/plan/MetadataDeleteNode.java (88%) diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/BasePlanFragmenter.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/BasePlanFragmenter.java index 348e031539142..927f6ca7e6a11 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/BasePlanFragmenter.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/BasePlanFragmenter.java @@ -25,6 +25,7 @@ import com.facebook.presto.spi.TableHandle; import com.facebook.presto.spi.VariableAllocator; import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.Partitioning; import com.facebook.presto.spi.plan.PartitioningHandle; @@ -42,7 +43,6 @@ import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.facebook.presto.sql.planner.plan.ExchangeNode; import com.facebook.presto.sql.planner.plan.ExplainAnalyzeNode; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.RemoteSourceNode; import com.facebook.presto.sql.planner.plan.SequenceNode; import com.facebook.presto.sql.planner.plan.SimplePlanRewriter; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java index 757c35e9854a2..1c6659466521f 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java @@ -156,6 +156,7 @@ import com.facebook.presto.spi.plan.JoinNode; import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.MarkDistinctNode; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OrderingScheme; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.PartitioningScheme; @@ -206,7 +207,6 @@ import com.facebook.presto.sql.planner.plan.GroupIdNode; import com.facebook.presto.sql.planner.plan.IndexJoinNode; import com.facebook.presto.sql.planner.plan.InternalPlanVisitor; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.RemoteSourceNode; import com.facebook.presto.sql.planner.plan.RowNumberNode; import com.facebook.presto.sql.planner.plan.SampleNode; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlanFragmenterUtils.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlanFragmenterUtils.java index 5f267f4c94e22..41c0f15975284 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlanFragmenterUtils.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlanFragmenterUtils.java @@ -22,6 +22,7 @@ import com.facebook.presto.spi.PrestoWarning; import com.facebook.presto.spi.TableHandle; import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.Partitioning; import com.facebook.presto.spi.plan.PartitioningHandle; @@ -32,7 +33,6 @@ import com.facebook.presto.spi.plan.TableScanNode; import com.facebook.presto.spi.plan.TableWriterNode; import com.facebook.presto.sql.planner.plan.ExplainAnalyzeNode; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.SimplePlanRewriter; import com.facebook.presto.sql.planner.plan.StatisticsWriterNode; import com.facebook.presto.sql.planner.plan.TableWriterMergeNode; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/SplitSourceFactory.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/SplitSourceFactory.java index 85e28b7679f4f..42c77c6758a4a 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/SplitSourceFactory.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/SplitSourceFactory.java @@ -27,6 +27,7 @@ import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.MarkDistinctNode; import com.facebook.presto.spi.plan.MergeJoinNode; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.plan.PlanNodeId; @@ -53,7 +54,6 @@ import com.facebook.presto.sql.planner.plan.GroupIdNode; import com.facebook.presto.sql.planner.plan.IndexJoinNode; import com.facebook.presto.sql.planner.plan.InternalPlanVisitor; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.RemoteSourceNode; import com.facebook.presto.sql.planner.plan.RowNumberNode; import com.facebook.presto.sql.planner.plan.SampleNode; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/MetadataDeleteOptimizer.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/MetadataDeleteOptimizer.java index 2d132f80d6842..c388ee2d4ca21 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/MetadataDeleteOptimizer.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/MetadataDeleteOptimizer.java @@ -18,13 +18,13 @@ import com.facebook.presto.spi.VariableAllocator; import com.facebook.presto.spi.WarningCollector; import com.facebook.presto.spi.plan.DeleteNode; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.plan.PlanNodeIdAllocator; import com.facebook.presto.spi.plan.TableFinishNode; import com.facebook.presto.spi.plan.TableScanNode; import com.facebook.presto.sql.planner.TypeProvider; import com.facebook.presto.sql.planner.plan.ExchangeNode; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.SimplePlanRewriter; import com.google.common.collect.Iterables; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/InternalPlanVisitor.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/InternalPlanVisitor.java index 878d55b024fc7..15b53e87ac787 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/InternalPlanVisitor.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/InternalPlanVisitor.java @@ -52,11 +52,6 @@ public R visitTableWriteMerge(TableWriterMergeNode node, C context) return visitPlan(node, context); } - public R visitMetadataDelete(MetadataDeleteNode node, C context) - { - return visitPlan(node, context); - } - public R visitUpdate(UpdateNode node, C context) { return visitPlan(node, context); diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java index b5cbaf20a71bb..8de8906b60d60 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java @@ -51,6 +51,7 @@ import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.MarkDistinctNode; import com.facebook.presto.spi.plan.MergeJoinNode; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OrderingScheme; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.Partitioning; @@ -90,7 +91,6 @@ import com.facebook.presto.sql.planner.plan.IndexJoinNode; import com.facebook.presto.sql.planner.plan.InternalPlanVisitor; import com.facebook.presto.sql.planner.plan.LateralJoinNode; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.RemoteSourceNode; import com.facebook.presto.sql.planner.plan.RowNumberNode; import com.facebook.presto.sql.planner.plan.SampleNode; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java index 2a658708324f7..8cc69302e7023 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/planner/sanity/ValidateDependenciesChecker.java @@ -32,6 +32,7 @@ import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.MarkDistinctNode; import com.facebook.presto.spi.plan.MergeJoinNode; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.PlanNode; import com.facebook.presto.spi.plan.ProjectNode; @@ -62,7 +63,6 @@ import com.facebook.presto.sql.planner.plan.IndexJoinNode; import com.facebook.presto.sql.planner.plan.InternalPlanVisitor; import com.facebook.presto.sql.planner.plan.LateralJoinNode; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.OffsetNode; import com.facebook.presto.sql.planner.plan.RemoteSourceNode; import com.facebook.presto.sql.planner.plan.RowNumberNode; diff --git a/presto-main-base/src/main/java/com/facebook/presto/util/GraphvizPrinter.java b/presto-main-base/src/main/java/com/facebook/presto/util/GraphvizPrinter.java index f05239ef8d418..882b0c6e16d01 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/util/GraphvizPrinter.java +++ b/presto-main-base/src/main/java/com/facebook/presto/util/GraphvizPrinter.java @@ -31,6 +31,7 @@ import com.facebook.presto.spi.plan.LimitNode; import com.facebook.presto.spi.plan.MarkDistinctNode; import com.facebook.presto.spi.plan.MergeJoinNode; +import com.facebook.presto.spi.plan.MetadataDeleteNode; import com.facebook.presto.spi.plan.OutputNode; import com.facebook.presto.spi.plan.PlanFragmentId; import com.facebook.presto.spi.plan.PlanNode; @@ -60,7 +61,6 @@ import com.facebook.presto.sql.planner.plan.IndexJoinNode; import com.facebook.presto.sql.planner.plan.InternalPlanVisitor; import com.facebook.presto.sql.planner.plan.LateralJoinNode; -import com.facebook.presto.sql.planner.plan.MetadataDeleteNode; import com.facebook.presto.sql.planner.plan.RemoteSourceNode; import com.facebook.presto.sql.planner.plan.RowNumberNode; import com.facebook.presto.sql.planner.plan.SampleNode; diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/MetadataDeleteNode.java b/presto-spi/src/main/java/com/facebook/presto/spi/plan/MetadataDeleteNode.java similarity index 88% rename from presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/MetadataDeleteNode.java rename to presto-spi/src/main/java/com/facebook/presto/spi/plan/MetadataDeleteNode.java index 1f35f4bb2e2fb..7d970515197ae 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/planner/plan/MetadataDeleteNode.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/plan/MetadataDeleteNode.java @@ -11,19 +11,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.facebook.presto.sql.planner.plan; +package com.facebook.presto.spi.plan; import com.facebook.presto.spi.SourceLocation; import com.facebook.presto.spi.TableHandle; -import com.facebook.presto.spi.plan.PlanNode; -import com.facebook.presto.spi.plan.PlanNodeId; import com.facebook.presto.spi.relation.VariableReferenceExpression; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.collect.ImmutableList; import javax.annotation.concurrent.Immutable; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -31,7 +29,7 @@ @Immutable public class MetadataDeleteNode - extends InternalPlanNode + extends PlanNode { private final TableHandle tableHandle; private final VariableReferenceExpression output; @@ -74,17 +72,17 @@ public VariableReferenceExpression getOutput() @Override public List getOutputVariables() { - return ImmutableList.of(output); + return Collections.singletonList(output); } @Override public List getSources() { - return ImmutableList.of(); + return Collections.emptyList(); } @Override - public R accept(InternalPlanVisitor visitor, C context) + public R accept(PlanVisitor visitor, C context) { return visitor.visitMetadataDelete(this, context); } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/plan/PlanVisitor.java b/presto-spi/src/main/java/com/facebook/presto/spi/plan/PlanVisitor.java index d39a5b1df8357..f0fefddc7a1f0 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/plan/PlanVisitor.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/plan/PlanVisitor.java @@ -154,4 +154,9 @@ public R visitUnnest(UnnestNode node, C context) { return visitPlan(node, context); } + + public R visitMetadataDelete(MetadataDeleteNode node, C context) + { + return visitPlan(node, context); + } }