Skip to content

fix(iceberg): fix table creation without namespace location #26744

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+3 −1
Open

fix(iceberg): fix table creation without namespace location #26744

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fa45bc
fix(iceberg): fix table creation without namespace location
twuebi Sep 29, 2025
1197870
undo accidental newline
twuebi Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1284,7 +1284,9 @@ public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Con
Location location = Location.of(transaction.table().location());
try {
// S3 Tables internally assigns a unique location for each table
if (!isS3Tables(location.toString())) {
// we create a non-staged table if tableLocation.isEmpty(), in that case, the table location will not be
// empty
if (!isS3Tables(location.toString()) && !(tableLocation != null && tableLocation.isEmpty())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about non-zero length whitespaced string : " " or " " ?
should we also add tableLocation.isBlank() e.g.

if (!isS3Tables(location.toString()) && !(tableLocation != null && tableLocation.isEmpty() && tableLocation.isBlank())) {

Copy link
Author

Choose a reason for hiding this comment

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

Can add those, wondering if location should reject such invalid location strings?

Copy link
Author

Choose a reason for hiding this comment

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

TBH, this all looks a bit hacky, ideally, we'd know if a table's creation was staged or not and only verify the location's emptiness based on that

TrinoFileSystem fileSystem = fileSystemFactory.create(session.getIdentity(), transaction.table().io().properties());
if (!replace && fileSystem.listFiles(location).hasNext()) {
throw new TrinoException(ICEBERG_FILESYSTEM_ERROR, format("" +
Expand Down
Loading