Skip to content

Catch all "not found" exceptions#19693

Merged
findepi merged 1 commit intomasterfrom
findepi/catch-all-not-found-exceptions-fc5c92
Nov 20, 2023
Merged

Catch all "not found" exceptions#19693
findepi merged 1 commit intomasterfrom
findepi/catch-all-not-found-exceptions-fc5c92

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Nov 9, 2023

Table/view not found can be indicated by TrinoException with TABLE_NOT_FOUND error code, but also by TableNotFoundException or ViewNotFoundException exceptions (both using NOT_FOUND error code). The code catching "not found" exceptions should catch all three.

@cla-bot cla-bot bot added the cla-signed label Nov 9, 2023
@github-actions github-actions bot added tests:hive iceberg Iceberg connector hive Hive connector labels Nov 9, 2023
@findepi findepi force-pushed the findepi/catch-all-not-found-exceptions-fc5c92 branch from 29dddbd to 8e1a84f Compare November 13, 2023 10:34
@findepi findepi requested a review from a team November 13, 2023 10:34
}
catch (TrinoException e) {
if (e.getErrorCode().equals(TABLE_NOT_FOUND.toErrorCode())) {
if (e.getErrorCode().equals(TABLE_NOT_FOUND.toErrorCode()) || e instanceof TableNotFoundException || e instanceof ViewNotFoundException) {
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.

why checking exception instance. Can you look check if error code is either TABLE_NOT_FOUND or NOT_FOUND.

Also can we unify? Are there many places which still throw exception with TABLE_NOT_FOUND?

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.

generally NOT_FOUND feels too broad (eg ColumnNotFoundException, SchemaNotFoundException, ...)

Also can we unify?

this idk

Table/view not found can be indicated by `TrinoException` with
`TABLE_NOT_FOUND` error code, but also by `TableNotFoundException` or
`ViewNotFoundException` exceptions (both using `NOT_FOUND` error code).
The code catching "not found" exceptions should catch all three.
@findepi findepi force-pushed the findepi/catch-all-not-found-exceptions-fc5c92 branch from f35cd89 to 1f26b04 Compare November 20, 2023 11:41
@findepi findepi merged commit 0d2dc48 into master Nov 20, 2023
@findepi findepi deleted the findepi/catch-all-not-found-exceptions-fc5c92 branch November 20, 2023 17:27
@github-actions github-actions bot added this to the 434 milestone Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed hive Hive connector iceberg Iceberg connector

Development

Successfully merging this pull request may close these issues.

2 participants