-
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
Prevent table creation on non-empty location for Iceberg tables #12626
Prevent table creation on non-empty location for Iceberg tables #12626
Conversation
HdfsContext hdfsContext = new HdfsContext(session); | ||
try { | ||
FileSystem fileSystem = hdfsEnvironment.getFileSystem(hdfsContext, new Path(location)); | ||
if (fileSystem.exists(new Path(location))) { |
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.
If the directory exists, but is empty. Should we allow it? That seems reasonable to me
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.
That can be useful and indeed is safe to do.
(except for concurrency, which we need to ignore anyway)
ac8aa17
to
4ca1941
Compare
retryMode); | ||
} | ||
catch (IOException e) { | ||
throw new TrinoException(ICEBERG_FILESYSTEM_ERROR, "Failed getting FileSystem: " + location, e); |
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.
it's not only hdfsEnvironment.getFileSystem(
what can throw IOE.
change message to
"Failed checking new table's location: " + location
throw new TrinoException(ICEBERG_FILESYSTEM_ERROR, "Cannot create a table on a non-empty location: " + location + ", " + | ||
"set 'iceberg.unique-table-location=true' in your Iceberg catalog properties to use unique table locations for every table."); |
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.
use format
throw new TrinoException(ICEBERG_FILESYSTEM_ERROR, "Cannot create a table on a non-empty location: " + location + ", " + | |
"set 'iceberg.unique-table-location=true' in your Iceberg catalog properties to use unique table locations for every table."); | |
throw new TrinoException(ICEBERG_FILESYSTEM_ERROR, format( | |
"Cannot create a table on a non-empty location: %s, set 'iceberg.unique-table-location=true' in your Iceberg catalog properties " + | |
"to use unique table locations for every table.", | |
location)); |
971bdd0
to
512e4bf
Compare
512e4bf
to
cb4b583
Compare
Description
Fix
Change to Iceberg connector
Related issues, pull requests, and links
CREATE TABLE
statement allows to create a table on a location already used by another Iceberg table #12573Documentation
( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
( ) Release notes entries required with the following suggested text: