Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DeleteNode to presto_protocol serialization #24274

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ghelmling
Copy link
Contributor

Description

This adds basic support for DeleteNode to presto_protocol serialization. This is required for DELETE statement plans to be passed through to Prestissimo workers.

Motivation and Context

This is the first step in adding DELETE query plan support to Prestissimo/Velox, by supporting basic serialization of the Java com.facebook.presto.spi.plan.DeleteNode class and members. While this includes basic (de)serialization, the generated protocol::DeleteNode class is not yet hooked up to any connectors. Conversion to Velox Hive Connector types will follow in a subsequent PR.

Impact

DeleteNode has been added to presto_protocol so it is now available for use by Prestissimo connector implementations.

Test Plan

A basic test, DeleteTest.cpp, is included, which validates round-trip serialization of a sample DeleteNode representation.

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 ==

@aditi-pandit
Copy link
Contributor

@ghelmling : Thanks for this change.

Quick question : Do you have a design for this work ? Would be great to read about it.

Please can you separate the velox changes out of this PR. We typically push PRs that only "Advance Velox" e.g. #24135

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants