Improve Iceberg system table handling #8692
Conversation
findepi
force-pushed
the
findepi/iceberg-system-table-handling
branch
2 times, most recently
from
cb9361d to
0f3aa08
Compare
...t-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergHiveTablesCompatibility.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergSystemTables.java
Outdated
Show resolved
Hide resolved
|
CI #8691 & a related failure. |
findepi
force-pushed
the
findepi/iceberg-system-table-handling
branch
from
d6835a4 to
be6bc6e
Compare
| name = IcebergTableName.from(tableName.getTableName()); | ||
| } | ||
| catch (InvalidIcebergTableName e) { | ||
| log.debug(e, "Invalid table name when looking for a table"); |
There was a problem hiding this comment.
I don't think this behavior is useful. Let's change IcebergTableName.from() to return Optional and maybe rename it to parse(). The only reason we threw exceptions was to present them to the end user.
There was a problem hiding this comment.
I realized the exceptions were thoughtfully constructed, and so i didn;'t want to just discard those messages.
But then, presenting them directly to a user would be at odds with "table not found" expected behavior.
That's why i left them as debug output, in case there is any need to debug anything.
In the future, we can emit them as warnings. If it makes sense to you as well, i will add a TODO in the code.
There was a problem hiding this comment.
They were thoughtfully constructed because they were presented to end users. Since these are no longer displayed anywhere, I'd rather just remove the code.
There was a problem hiding this comment.
Actually, I'm not convinced that NOT_FOUND is necessarily the right behavior here. These are hidden tables, so there's nothing preventing us from saying that they do exist but are invalid. These would be similar to a Hive table that exists but has bad metadata, for which we would throw HIVE_INVALID_METADATA. Or how we throw HIVE_UNSUPPORTED_FORMAT for Delta Lake tables.
findepi
force-pushed
the
findepi/iceberg-system-table-handling
branch
from
36e1e25 to
71feabb
Compare
findepi
force-pushed
the
findepi/iceberg-system-table-handling
branch
from
71feabb to
8524e1e
Compare
|
CI #8691 |
Fixes #8690
Fixes #5459
Also for #8675 (#8675 (comment))