Correct exception type when table exists while creating a view#15580
Correct exception type when table exists while creating a view#15580krvikash wants to merge 1 commit intotrinodb:masterfrom
Conversation
krvikash
added
the
no-release-notes
krvikash
force-pushed
the
fix-error-message-when-table-exists-while-creating-view
branch
2 times, most recently
from
785bfd4 to
8451595
Compare
krvikash
changed the title
krvikash
force-pushed
the
fix-error-message-when-table-exists-while-creating-view
branch
from
8451595 to
50e0178
Compare
|
nit:
|
There was a problem hiding this comment.
There are other uses of ViewAlreadyExistsException which may be probably replaced by TableAlreadyExistsException.
If you replace all of them, then probably ViewAlreadyExistsException may be removed.
cc @electrum
There was a problem hiding this comment.
ViewAlreadyExistsException is required for the case when a view already exists while creating a view.
This PR specially deals with when A table exists while creating a view.
There was a problem hiding this comment.
In the cases where currently ViewAlreadyExistsException we are doing previously metastore.getTable(..) calls.
I'm thinking that if the metastore calls for a "table", then TableAlreadyExistsException is appropriate.
ViewAlreadyExistsException exception is misleading when a table exists while creating a view. Changing it to TableAlreadyExistsException.
krvikash
force-pushed
the
fix-error-message-when-table-exists-while-creating-view
branch
from
50e0178 to
c6b98ea
Compare
|
The |
| doCreateView(temporaryCreateView, false); | ||
| fail("create existing should fail"); | ||
| } | ||
| catch (TableAlreadyExistsException e) { |
There was a problem hiding this comment.
shall we remove the catch (ViewAlreadyExistsExcept clause below?
| Optional<com.amazonaws.services.glue.model.Table> existing = getTable(session, schemaViewName); | ||
| if (existing.isPresent()) { | ||
| if (!replace || !isPrestoView(firstNonNull(existing.get().getParameters(), ImmutableMap.of()))) { | ||
| // TODO: ViewAlreadyExists is misleading if the name is used by a table https://github.com/trinodb/trino/issues/10037 |
There was a problem hiding this comment.
is this PR supposed to close that issue?
There was a problem hiding this comment.
I think Issue #10037 should have been closed by #10186.
This PR #15580 is follow-up TODO task for https://github.com/trinodb/trino/pull/11499/files#diff-0c09267905a13855768761575aee308f9a1773e6f3d59c6347de78fa8dbc1c02R422
|
@krvikash please rebase, there is a conflict |
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
@krvikash, is this PR still active? Could you rebase this please? |
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
Closing this one out due to inactivity, but please reopen if you would like to pick this back up. |
6 participants
Description
ViewAlreadyExistsExceptionexception is misleading when a table exists while creating a view. Changing it toTableAlreadyExistsException.Release notes
(X) This is not user-visible or docs only and no release notes are required.