-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Added ability to recursively delete table data in Iceberg #6108
Conversation
…able data and metadata from storage. This PR adding ability to also recursively removes table data and metadata from storage. Added new configuration property which is responsible for enabling/disabling recursive delete of table data on drop.
As we discussed in #5616 I have created this PR which is adding ability to recursively delete table data on table drop action |
# Conflicts: # presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/IcebergConfig.java # presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/IcebergMetadata.java # presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/IcebergMetadataFactory.java # presto-iceberg/src/test/java/io/prestosql/plugin/iceberg/TestIcebergConfig.java
@sshkvar can you please squash commits and rebase? |
} | ||
|
||
public IcebergMetadataFactory( | ||
HiveMetastore metastore, | ||
HdfsEnvironment hdfsEnvironment, | ||
TypeManager typeManager, | ||
JsonCodec<CommitTaskData> commitTaskCodec) | ||
JsonCodec<CommitTaskData> commitTaskCodec, | ||
boolean purgeTableData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
boolean purgeTableData) | |
boolean purgeTableDataOnDrop) |
Optional<Table> table = metastore.getTable(new HiveIdentity(session), handle.getSchemaName(), handle.getTableName()); | ||
|
||
metastore.dropTable(new HiveIdentity(session), handle.getSchemaName(), handle.getTableName(), false); | ||
if (purgeDataOnDrop && table.isPresent()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if drop is in the middle of a transaction and if it is aborted ? Deleting these files has to be done only when we commit those changes.
In Iceberg (ie within https://github.com/apache/iceberg) this seems to be dependent on the Catalog implementation.
I do think Trino does that now, doesn't it? |
I think we can close this as this functionality is already implemented from all I know. Can someone like @findepi @findinpath @waraposn or @Praveen2112 confirm. |
Closing the PR because the functionality has been already implemented in #11062 |
Iceberg removes table metadata only from metastore, but not removes table data and metadata from storage. This PR adding ability to also recursively removes table data and metadata from storage. Added new configuration property which is responsible for enabling/disabling recursive delete of table data on drop.