HIVE-29036: Support ViewCatalog through Iceberg REST API #5888
Conversation
okumin
force-pushed
the
HIVE-29036-rest-view
branch
from
62c44fb to
59fa33f
Compare
| catalog instanceof SupportsNamespaces ? (SupportsNamespaces) catalog : null; | ||
| this.asViewCatalog = catalog instanceof ViewCatalog ? (ViewCatalog) catalog : null; | ||
| this.asNamespaceCatalog = (SupportsNamespaces) catalog; | ||
| this.asViewCatalog = (ViewCatalog) catalog; |
There was a problem hiding this comment.
I believe our adapter does not need to be so generic
| RENAME_VIEW(HTTPMethod.POST, "v1/views/rename", | ||
| RenameTableRequest.class, null), | ||
| DROP_VIEW(HTTPMethod.DELETE, VIEWS_PATH); | ||
| TOKENS(HTTPMethod.POST, "v1/oauth/tokens", null), |
There was a problem hiding this comment.
I copied & pasted 1.9.1 of the upstream and removed unused fields. When our /v1/config returns the list of endpoints, the original form, such as v1/namespaces, doesn't work. We need to be adaptable to the new form such as /v1/{prefix}/namespaces.
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
There was a problem hiding this comment.
what is the role of {prefix} - catalogName aka iceberg?
There was a problem hiding this comment.
It is an optional path parameter, which we can freely use.
apache/iceberg#5233
S3 Tables use it to put a bucket identifier.
https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-tables-integrating-open-source.html#endpoint-parameter
I guess multi-tenant services utilize it, but I've not used it so far.
| return castResponse(ConfigResponse.class, ConfigResponse.builder().build()); | ||
| final List<Endpoint> endpoints = Arrays.stream(Route.values()) | ||
| .map(r -> Endpoint.create(r.method.name(), r.resourcePath)).collect(Collectors.toList()); | ||
| return castResponse(ConfigResponse.class, ConfigResponse.builder().withEndpoints(endpoints).build()); |
There was a problem hiding this comment.
This change is one of the key points of this PR. The Iceberg client doesn't support views unless we explicitly declare that we support view endpoints.
#5887 (comment)
okumin
changed the title
asf-ci-hive
added
tests passed
tests pending
tests unstable
and removed
tests pending
tests passed
labels
okumin
force-pushed
the
HIVE-29036-rest-view
branch
from
59fa33f to
585eebb
Compare
asf-ci-hive
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
| catalog instanceof SupportsNamespaces ? (SupportsNamespaces) catalog : null; | ||
| this.asViewCatalog = catalog instanceof ViewCatalog ? (ViewCatalog) catalog : null; | ||
| this.asNamespaceCatalog = (SupportsNamespaces) catalog; | ||
| this.asViewCatalog = (ViewCatalog) catalog; | ||
| } | ||
|
|
||
| enum HTTPMethod { |
There was a problem hiding this comment.
@okumin, could you please drop this enum and import org.apache.iceberg.rest.HTTPRequest.HTTPMethod?
|
What changes were proposed in this pull request?
Add the view support with integration tests.
https://issues.apache.org/jira/browse/HIVE-29036
Why are the changes needed?
With the current configuration, the Iceberg client ignores view-related endpoints.
Does this PR introduce any user-facing change?
No. REST Catalog has not been released yet.
How was this patch tested?