Core: Add View support for REST catalog by nastra · Pull Request #7913 · apache/iceberg

nastra · 2023-06-26T13:12:44Z

No description provided.

core/src/main/java/org/apache/iceberg/rest/requests/UpdateViewRequest.java

open-api/rest-catalog-open-api.yaml

nastra · 2023-11-08T15:30:11Z

core/src/main/java/org/apache/iceberg/view/ViewMetadata.java


-    private int addVersionInternal(ViewVersion version) {
-      int newVersionId = reuseOrCreateNewViewVersionId(version);
+    private int addVersionInternal(ViewVersion newVersion) {


I've renamed this parameter to make the code slightly easier to understand, because we have a few cases in this method where we re-assign either the schema id or the view version id

core/src/main/java/org/apache/iceberg/view/ViewMetadata.java

rdblue · 2023-12-05T00:15:07Z

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java

                    .create())
-        .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("Invalid view version: Cannot add multiple queries for dialect trino");
+        .isInstanceOf(Exception.class)


Minor: I think I pointed this out before and may have forgotten the answer, but shouldn't this be a consistent exception class every time?

I remember I replied to this but somehow can't find where it was.
The issue with the REST version is unfortunately that it fails with org.apache.iceberg.exceptions.BadRequestException: Malformed request: Invalid view version: Cannot add multiple queries for dialect trino because we're re-applying all changes to the Builder in CatalogHandlers.createView(...):

iceberg/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java

Lines 424 to 429 in f076b4d

request.viewVersion().representations().stream()

.filter(SQLViewRepresentation.class::isInstance)

.map(SQLViewRepresentation.class::cast)

.forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql()));

View view = viewBuilder.create();

That IAE is then converted to a BadRequestException in

iceberg/core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java

Lines 204 to 205 in ace0b13

case 400:

throw new BadRequestException("Malformed request: %s", error.message());

So I think what we could do is to have

if (IllegalArgumentException.class.getSimpleName().equals(error.type())) { throw new IllegalArgumentException(error.message()); }

in the DefaultErrorHandler. Thoughts?

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java

rdblue

I think this is nearly ready but the concurrent test case actually refreshes correctly and doesn't test the concurrent case with server-side retry and ID reassignment. I made a suggestion in the comment that I think will fix the test issue.

This will probably also require a flag in ViewCatalogTests for when server-side retry is possible like in table tests. If we're testing it correctly, then the InMemoryCatalog will fail.

rdblue · 2023-12-05T16:03:58Z

Thanks @nastra! Great to have this in.

(cherry picked from commit f19643a)

nastra marked this pull request as draft June 26, 2023 13:12

github-actions bot added API core labels Jun 26, 2023

nastra force-pushed the rest-view-support branch 3 times, most recently from 16eb260 to d2d5be6 Compare June 26, 2023 14:52

rdblue reviewed Jun 26, 2023

View reviewed changes

core/src/main/java/org/apache/iceberg/rest/requests/UpdateViewRequest.java Outdated Show resolved Hide resolved

nastra force-pushed the rest-view-support branch from d2d5be6 to 1c9df82 Compare June 28, 2023 09:32

github-actions bot added the OPENAPI label Jun 28, 2023

nastra force-pushed the rest-view-support branch 4 times, most recently from 045f67d to dd6eac0 Compare June 29, 2023 12:30

nastra requested a review from danielcweeks June 29, 2023 15:09

danielcweeks reviewed Jun 29, 2023

View reviewed changes

open-api/rest-catalog-open-api.yaml Show resolved Hide resolved

nastra mentioned this pull request Jul 6, 2023

Core: Simplify/improve View APIs #7992

Merged

nastra force-pushed the rest-view-support branch 3 times, most recently from 75dfd77 to b62b93d Compare July 13, 2023 07:58

nastra mentioned this pull request Jul 25, 2023

Core: Add metadata updates for Views / Use own Builder for ViewMetadata #8147

Merged

nastra force-pushed the rest-view-support branch from b62b93d to 1396eb1 Compare August 3, 2023 16:25

nastra added this to the Iceberg 1.4.0 milestone Aug 9, 2023

JanKaul mentioned this pull request Sep 1, 2023

feat: Add Catalog API apache/iceberg-rust#54

Merged

Xuanwo mentioned this pull request Sep 1, 2023

Iceberg View Support apache/iceberg-rust#55

Open

nastra force-pushed the rest-view-support branch 3 times, most recently from 50a65bb to 59bde7f Compare September 13, 2023 09:14

nastra marked this pull request as ready for review September 13, 2023 09:14

nastra force-pushed the rest-view-support branch from 59bde7f to ac36663 Compare September 14, 2023 09:29

danielcweeks removed this from the Iceberg 1.4.0 milestone Sep 19, 2023

nastra force-pushed the rest-view-support branch from 3d01bb9 to 626dedc Compare November 2, 2023 10:02

nastra added 4 commits November 7, 2023 17:31

Core: Add View support for REST catalog

ace0b13

review feedback

53104a0

requirement on view uuid

234b61c

review feedback

f076b4d

nastra force-pushed the rest-view-support branch 5 times, most recently from 9905345 to 9fb62ee Compare November 8, 2023 15:14

handle last added schema id

30753ba

nastra force-pushed the rest-view-support branch from 9fb62ee to 30753ba Compare November 8, 2023 15:17

nastra commented Nov 8, 2023

View reviewed changes

nastra mentioned this pull request Nov 9, 2023

Core: Improve view/table detection when replacing a table/view #9012

Merged

rdblue reviewed Dec 5, 2023

View reviewed changes

core/src/main/java/org/apache/iceberg/view/ViewMetadata.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 5, 2023

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Outdated Show resolved Hide resolved

rdblue reviewed Dec 5, 2023

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Show resolved Hide resolved

rdblue reviewed Dec 5, 2023

View reviewed changes

core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java Show resolved Hide resolved

rdblue requested changes Dec 5, 2023

View reviewed changes

improvements

d457c0b

rdblue approved these changes Dec 5, 2023

View reviewed changes

rdblue merged commit f19643a into apache:main Dec 5, 2023

nastra deleted the rest-view-support branch December 5, 2023 17:03

nastra mentioned this pull request Dec 5, 2023

Core: Handle IAE in default error handler #9225

Merged

lisirrx pushed a commit to lisirrx/iceberg that referenced this pull request Jan 4, 2024

Core: Add View support for REST catalog (apache#7913)

8e77ddb

devangjhabakh added a commit to cdouglas/iceberg that referenced this pull request Apr 22, 2024

Core: Add View support for REST catalog (apache#7913)

8dfa074

nastra self-assigned this Nov 25, 2024

zhongyujiang pushed a commit to zhongyujiang/iceberg that referenced this pull request Apr 16, 2025

Core: Add View support for REST catalog (apache#7913)

a9ae54d

(cherry picked from commit f19643a)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Core: Add View support for REST catalog#7913

Core: Add View support for REST catalog#7913
rdblue merged 6 commits intoapache:mainfrom
nastra:rest-view-support

nastra commented Jun 26, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

nastra Nov 8, 2023

Uh oh!

Uh oh!

rdblue Dec 5, 2023

Uh oh!

nastra Dec 5, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdblue left a comment

Uh oh!

rdblue commented Dec 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	request.viewVersion().representations().stream()
	.filter(SQLViewRepresentation.class::isInstance)
	.map(SQLViewRepresentation.class::cast)
	.forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql()));

	View view = viewBuilder.create();

	case 400:
	throw new BadRequestException("Malformed request: %s", error.message());

Comments

Conversation

nastra commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nastra Nov 8, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rdblue Dec 5, 2023

Choose a reason for hiding this comment

Uh oh!

nastra Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdblue left a comment

Choose a reason for hiding this comment

Uh oh!

rdblue commented Dec 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nastra commented Jun 26, 2023 •

edited

Loading

nastra Dec 5, 2023 •

edited

Loading