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

Core: Fix drop table without purge for REST session catalog #6511

Merged

Conversation

krvikash
Copy link
Contributor

@krvikash krvikash commented Jan 2, 2023

Issue:
As per documentation of SessionCataog.dropTable says

Drop a table, without requesting that files are immediately deleted.

But the RESTSessionCatalog#dropTable API purges the table even if we are not setting purgeRequested to true.

The issue is happening because, While handling DROP_TABLE request in RESTCatalogAdapter, the call goes to CatalogHandlers.dropTable(Catalog catalog, TableIdentifier ident) when purgeRequested is not set and internally calls catalog.dropTable(ident) that always drops the table with purge.

public static void dropTable(Catalog catalog, TableIdentifier ident) {
boolean dropped = catalog.dropTable(ident);
if (!dropped) {
throw new NoSuchTableException("Table does not exist: %s", ident);
}
}

Fix:
Fix CatalogHandlers.dropTable(Catalog catalog, TableIdentifier ident) API that drops the table without purge.

This PR is to fix the dropTable API for REST Session Catalog for dropping table without purging.

@github-actions github-actions bot added the core label Jan 2, 2023
@krvikash krvikash force-pushed the fix-drop-table-without-purge-rest-catalog branch from f77009e to 90c6f11 Compare January 2, 2023 09:37
@krvikash krvikash changed the title Fix drop table without purge for REST session catalog Core: Fix drop table without purge for REST session catalog Jan 2, 2023
Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catching this, thanks @krvikash

Copy link
Contributor

@singhpk234 singhpk234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch !!

@Fokko Fokko merged commit 50c736b into apache:master Jan 5, 2023
@krvikash krvikash deleted the fix-drop-table-without-purge-rest-catalog branch January 5, 2023 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants