Skip to content

Don't delete Hive and Iceberg schema locations with nonempty subdirectories#10485

Merged
losipiuk merged 2 commits intotrinodb:masterfrom
jirassimok:drop-schema-files-fixup
Jan 12, 2022
Merged

Don't delete Hive and Iceberg schema locations with nonempty subdirectories#10485
losipiuk merged 2 commits intotrinodb:masterfrom
jirassimok:drop-schema-files-fixup

Conversation

@jirassimok
Copy link
Copy Markdown
Member

@jirassimok jirassimok commented Jan 6, 2022

This fixes an error in #10146 and #9767 that caused files in subdirectories to be ignored when determining whether to delete schema locations when dropping Hive and Iceberg schemas.

(Any empty subdirectories will still be deleted with the schema, though.)

@cla-bot cla-bot bot added the cla-signed label Jan 6, 2022
@jirassimok jirassimok force-pushed the drop-schema-files-fixup branch 3 times, most recently from 6b62bca to 15b8e81 Compare January 7, 2022 03:21
@jirassimok jirassimok requested a review from losipiuk January 7, 2022 21:08
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there case with EMPTY subdirectory also covered (that it does not prevent schema from being deleted)?

Copy link
Copy Markdown
Member Author

@jirassimok jirassimok Jan 10, 2022

Choose a reason for hiding this comment

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

I've changed that behavior so empty directories won't be deleted, and I added a test for that as well.

@jirassimok jirassimok force-pushed the drop-schema-files-fixup branch 3 times, most recently from 39e1966 to 11aea45 Compare January 10, 2022 21:05
Comment on lines 59 to 62
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These lines aren't actually changed; I just switched the order of this test and the next after renaming them, and they're so similar that git didn't realize.

@jirassimok jirassimok requested a review from losipiuk January 10, 2022 21:46
@jirassimok jirassimok force-pushed the drop-schema-files-fixup branch from 11aea45 to 6cd0c32 Compare January 10, 2022 21:52
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit move the comment to method body.

@losipiuk
Copy link
Copy Markdown
Member

@jirassimok error-prone-checks is failing.

Previously, files in subdirectories weren't considered when deciding
whether to delete Hive and Iceberg schema locations when the schema
was dropped.
@jirassimok jirassimok force-pushed the drop-schema-files-fixup branch from 6cd0c32 to 0b7fea6 Compare January 11, 2022 15:03
@losipiuk losipiuk merged commit 6e16db2 into trinodb:master Jan 12, 2022
@github-actions github-actions bot added this to the 369 milestone Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants