Refactor MetadataDeleteNode to spi Plan #25430

natashasehgal · 2025-06-24T19:59:33Z

Description

Refactor MetadataDeleteNode 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

Unit tests

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

facebook-github-bot · 2025-06-24T19:59:41Z

This pull request was exported from Phabricator. Differential Revision: D77249220

Summary: Rollback Plan: Differential Revision: D77249220

facebook-github-bot · 2025-06-24T20:18:16Z

This pull request was exported from Phabricator. Differential Revision: D77249220

Summary: Rollback Plan: Differential Revision: D77249220

facebook-github-bot · 2025-06-24T21:55:14Z

This pull request was exported from Phabricator. Differential Revision: D77249220

presto-spi/src/main/java/com/facebook/presto/spi/plan/MetadataDeleteNode.java

natashasehgal · 2025-06-24T22:20:13Z

presto-spi/src/main/java/com/facebook/presto/spi/plan/MetadataDeleteNode.java

 @Immutable
 public class MetadataDeleteNode
-        extends InternalPlanNode
+        extends PlanNode


@ghelmling InternalPlanNode access issue after switching package

natashasehgal · 2025-06-25T00:50:14Z

@ghelmling MetadataDeleteOptimizer.findNode() implementation contains ExchangeNode, which is also in package com.facebook.presto.sql.planner.plan. Any thoughts on how to use ExchangeNode in FB's Metalake optimizer?

A refactor similar to MetadataDeleteNode for ExchangeNode would be quite large

Edit: Discussed offline, the optimization can be done slightly differently, instead of using ExchangeNode

ghelmling · 2025-06-26T17:21:56Z

lgtm! Thanks for the change. You'll just need committer approval.

facebook-github-bot · 2025-06-26T18:09:08Z

@natashasehgal has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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

facebook-github-bot · 2025-06-26T20:40:15Z

This pull request was exported from Phabricator. Differential Revision: D77249220

facebook-github-bot · 2025-06-26T22:02:06Z

@natashasehgal has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

hantangwangd

Thanks for the refactoring, overall looks good to me, just one nit.

Besides, there seems to be a couple of typos in the commit message—it should be MetadataDeleteNode instead of MetadeleteNode.

presto-spi/src/main/java/com/facebook/presto/spi/plan/PlanVisitor.java

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

facebook-github-bot · 2025-06-27T05:54:40Z

This pull request was exported from Phabricator. Differential Revision: D77249220

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

facebook-github-bot · 2025-06-27T16:15:38Z

This pull request was exported from Phabricator. Differential Revision: D77249220

facebook-github-bot · 2025-06-27T17:25:34Z

@natashasehgal has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

natashasehgal · 2025-06-27T17:33:30Z

Thanks for the refactoring, overall looks good to me, just one nit.

Besides, there seems to be a couple of typos in the commit message—it should be MetadataDeleteNode instead of MetadeleteNode.

You're right updated these typos, and removed unnecessary function

facebook-github-bot · 2025-06-27T18:38:24Z

@natashasehgal has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

natashasehgal requested a review from a team as a code owner June 24, 2025 19:59

prestodb-ci added the from:Meta PR from Meta label Jun 24, 2025

facebook-github-bot added the fb-exported label Jun 24, 2025

natashasehgal added a commit to natashasehgal/presto that referenced this pull request Jun 24, 2025

Refactor MetadeleteNode to spi Plan (prestodb#25430)

21d5696

Summary: Rollback Plan: Differential Revision: D77249220

natashasehgal force-pushed the export-D77249220 branch from a08c457 to 21d5696 Compare June 24, 2025 20:18

natashasehgal added a commit to natashasehgal/presto that referenced this pull request Jun 24, 2025

Refactor MetadeleteNode to spi Plan (prestodb#25430)

ac0a774

Summary: Rollback Plan: Differential Revision: D77249220

natashasehgal force-pushed the export-D77249220 branch from 21d5696 to ac0a774 Compare June 24, 2025 21:55

natashasehgal commented Jun 24, 2025

View reviewed changes

presto-spi/src/main/java/com/facebook/presto/spi/plan/MetadataDeleteNode.java Show resolved Hide resolved

natashasehgal commented Jun 24, 2025

View reviewed changes

natashasehgal requested a review from hantangwangd June 26, 2025 18:02

natashasehgal force-pushed the export-D77249220 branch from 93d4248 to 7b23a46 Compare June 26, 2025 20:40

natashasehgal requested a review from feilong-liu June 26, 2025 22:56

hantangwangd requested changes Jun 27, 2025

View reviewed changes

presto-spi/src/main/java/com/facebook/presto/spi/plan/PlanVisitor.java Show resolved Hide resolved

natashasehgal force-pushed the export-D77249220 branch from 7b23a46 to ecfdcfb Compare June 27, 2025 05:54

natashasehgal changed the title ~~Refactor MetadeleteNode to spi Plan~~ Refactor MetadataDeleteNode to spi Plan Jun 27, 2025

natashasehgal force-pushed the export-D77249220 branch from ecfdcfb to dac3b01 Compare June 27, 2025 16:15

natashasehgal requested a review from hantangwangd June 27, 2025 17:33

hantangwangd approved these changes Jun 27, 2025

View reviewed changes

natashasehgal merged commit 72490a5 into prestodb:master Jun 27, 2025
107 of 108 checks passed

natashasehgal deleted the export-D77249220 branch June 27, 2025 18:26

This was referenced Jul 4, 2025

Add release notes for 0.294 unix280/presto#35

Merged

Add release notes for 0.294 unix280/presto#36

Closed

Add release notes for 0.294 unix280/presto#37

Closed

This was referenced Jul 24, 2025

Add release notes for 0.294 unix280/presto#39

Merged

Add release notes for 0.294 unix280/presto#40

Merged

prestodb-ci mentioned this pull request Jul 28, 2025

Add release notes for 0.294 #25633

Merged

6 tasks

Refactor MetadataDeleteNode to spi Plan #25430

Refactor MetadataDeleteNode to spi Plan #25430

Uh oh!

Conversation

natashasehgal commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

facebook-github-bot commented Jun 24, 2025

Uh oh!

facebook-github-bot commented Jun 24, 2025

Uh oh!

facebook-github-bot commented Jun 24, 2025

Uh oh!

Uh oh!

natashasehgal Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

natashasehgal commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghelmling commented Jun 26, 2025

Uh oh!

facebook-github-bot commented Jun 26, 2025

Uh oh!

facebook-github-bot commented Jun 26, 2025

Uh oh!

facebook-github-bot commented Jun 26, 2025

Uh oh!

hantangwangd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Jun 27, 2025

Uh oh!

facebook-github-bot commented Jun 27, 2025

Uh oh!

facebook-github-bot commented Jun 27, 2025

Uh oh!

natashasehgal commented Jun 27, 2025

Uh oh!

Uh oh!

facebook-github-bot commented Jun 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

natashasehgal commented Jun 24, 2025 •

edited

Loading

natashasehgal commented Jun 25, 2025 •

edited

Loading