Set existing location for view when replacing view for Iceberg Rest #26736
Conversation
There was a problem hiding this comment.
Please update the test and shorten the commit title: https://trino.io/development/process.html#pull-request-and-commit-guidelines-
codluca
force-pushed
the
25425-create-or-replace-view-unique-location-iceberg-rest
branch
2 times, most recently
from
1233e1b to
82b5c3b
Compare
|
Added a new test with useUniqueTableLocations=true and made the commit title shorter. |
codluca
force-pushed
the
25425-create-or-replace-view-unique-location-iceberg-rest
branch
2 times, most recently
from
f0c00e9 to
f8f7dae
Compare
|
|
||
| @Test | ||
| @Override | ||
| public void testViewUnique() |
There was a problem hiding this comment.
update name sth like testCreateReplaceViewUniqueLocation
There was a problem hiding this comment.
update name sth like
testCreateReplaceViewUniqueLocation
Fixed
| ViewBuilder viewBuilder = restSessionCatalog.buildView(convert(session), toRemoteView(session, schemaViewName, true)); | ||
| String tableLocation = defaultTableLocation(session, schemaViewName); | ||
| if (replace && useUniqueTableLocation) { | ||
| Optional<View> view = getIcebergView(session, schemaViewName, true); |
There was a problem hiding this comment.
will we fail to get the view if it isn't in the view cache?
There was a problem hiding this comment.
will we fail to get the view if it isn't in the view cache?
I believe it will attempt to return from the cache, and if missing it will load it.
It reaches CacheUtils.uncheckedCacheGet, where it does "return cache.get(key, loader::get);"
codluca
force-pushed
the
25425-create-or-replace-view-unique-location-iceberg-rest
branch
from
17f408d to
30ffe4c
Compare
| try { | ||
| catalog.dropNamespace(SESSION, namespace); | ||
| } | ||
| catch (Exception e) { |
| .doesNotContain(renamedSchemaTableName); | ||
| } | ||
| finally { | ||
| try { |
| String namespace = "test_create_view_" + randomNameSuffix(); | ||
| String viewName = "viewName"; | ||
| String renamedViewName = "renamedViewName"; | ||
| SchemaTableName schemaTableName = new SchemaTableName(namespace, viewName); |
There was a problem hiding this comment.
Where is the logic to check the location is the same before and after the "create or replace"
There was a problem hiding this comment.
Couldn't find a way to get the view location, as that is a property from apache iceberg view (org.apache.iceberg.view).
Moved the test to TestTrinoRestCatalog, where I could inspect the temporary folder and extract the view name.
codluca
force-pushed
the
25425-create-or-replace-view-unique-location-iceberg-rest
branch
from
6927de5 to
8f5b537
Compare
If replace and useUniqueTableLocation, load the existing view and reuse its location.
codluca
force-pushed
the
25425-create-or-replace-view-unique-location-iceberg-rest
branch
from
8f5b537 to
9748f78
Compare
Set existing location for view when replacing view for Iceberg Rest catalog
Fixes #25425
Description
If replace and useUniqueTableLocation, load the existing view and reuse its location.
Additional context and related issues
Release notes
(X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: