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

Update begin delete #24528

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

Conversation

shelton408
Copy link

@shelton408 shelton408 commented Feb 11, 2025

Description

WIP - rough draft to check the potential scale of impact
Changes to separate delete table handles from generic table handles.

Motivation and Context

Please refer to #24520

Impact

Metadata, ConnectorMetadata, MetadataManager changed
->downstream affects execution: TableWriteInfo, ExecutionWriterTarget, TableWriterNode
Create 2 new types: ConnectorDeleteTableHandle, DeleteTableHandle
Types Changed: TableWriterNode.DeleteHandle

Also affects finishDelete, getDeleteRowIdColumnHandle types (AbstractMockMetadata, DelegatingMetadataManager)

Other classes affected:
ConnectorPageSinkProvider -> provide new pagesink for deletes?
ConnectorHandleResolver -> provide new getDeleteTableHandleClass method -> HandleResolver implements new ConnectorDeleteTableHandle

All connectors affected, those that implement DELETE will require more changes.
Kudu and iceberg connector both need to change types used for begin/finish delete
Iceberg currently uses IcebergTableHandle which inherits from basehivetablehandle->connectorTableHandle, and we would need to create a separate IcebergTableHandle for deletes

This would eventually apply to update as well if were going to make the code symmetric

Test Plan

TBD
Pass presto-icebergs tests (includes DELETE 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.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ... 

If release note is NOT required, use:

== NO RELEASE NOTE ==

@shelton408 shelton408 requested a review from a team as a code owner February 11, 2025 01:42
Copy link

linux-foundation-easycla bot commented Feb 11, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@shelton408
Copy link
Author

Has passed presto-Iceberg delete tests In the current state

Changes to update finishDelete to new types

Fixed Iceberg Delete as proof of concept. Fixed missing serialization changes.

Kudu fix and checkstyle

recombine iceberg tablehandle and deletetablehandle, fix serialization

Documentation and add delete pagesink

Fix implementation of delete pagesink
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.

1 participant