rdblue commented on code in PR #7913:
URL: https://github.com/apache/iceberg/pull/7913#discussion_r1342188931
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -971,4 +996,250 @@ public void commitTransaction(SessionContext context,
List<TableCommit> commits)
headers(context),
ErrorHandlers.tableCommitHandler());
}
+
+ @Override
+ public List<TableIdentifier> listViews(SessionContext context, Namespace
namespace) {
+ checkNamespaceIsValid(namespace);
+
+ ListTablesResponse response =
+ client.get(
+ paths.views(namespace),
+ ListTablesResponse.class,
+ headers(context),
+ ErrorHandlers.namespaceErrorHandler());
+ return response.identifiers();
+ }
+
+ @Override
+ public View loadView(SessionContext context, TableIdentifier identifier) {
+ checkViewIdentifierIsValid(identifier);
+
+ LoadViewResponse response =
+ client.get(
+ paths.view(identifier),
+ LoadViewResponse.class,
+ headers(context),
+ ErrorHandlers.viewErrorHandler());
+
+ AuthSession session = tableSession(response.config(), session(context));
+ ViewMetadata metadata = response.metadata();
+
+ RESTViewOperations ops =
+ new RESTViewOperations(
+ client,
+ paths.view(identifier),
+ session::headers,
+ tableFileIO(context, response.config()),
+ metadata);
+
+ trackFileIO(ops);
+
+ return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
+ }
+
+ @Override
+ public RESTViewBuilder buildView(SessionContext context, TableIdentifier
identifier) {
+ return new RESTViewBuilder(context, identifier);
+ }
+
+ @Override
+ public boolean dropView(SessionContext context, TableIdentifier identifier) {
+ checkViewIdentifierIsValid(identifier);
+
+ try {
+ client.delete(
+ paths.view(identifier), null, headers(context),
ErrorHandlers.viewErrorHandler());
+ return true;
+ } catch (NoSuchViewException e) {
+ return false;
+ }
+ }
+
+ @Override
+ public void renameView(SessionContext context, TableIdentifier from,
TableIdentifier to) {
+ checkViewIdentifierIsValid(from);
+ checkViewIdentifierIsValid(to);
+
+ RenameTableRequest request =
+
RenameTableRequest.builder().withSource(from).withDestination(to).build();
+
+ client.post(
+ paths.renameView(), request, null, headers(context),
ErrorHandlers.viewErrorHandler());
+ }
+
+ private class RESTViewBuilder implements ViewBuilder {
+ private final SessionContext context;
+ private final TableIdentifier identifier;
+ private final ImmutableViewVersion.Builder viewVersionBuilder =
ImmutableViewVersion.builder();
+ private final Map<String, String> properties = Maps.newHashMap();
+ private final List<ViewRepresentation> representations =
Lists.newArrayList();
+ private Namespace defaultNamespace = null;
+ private Schema schema = null;
+
+ private RESTViewBuilder(SessionContext context, TableIdentifier
identifier) {
+ checkViewIdentifierIsValid(identifier);
+ this.identifier = identifier;
+ this.context = context;
+ }
+
+ @Override
+ public ViewBuilder withSchema(Schema newSchema) {
+ this.schema = newSchema;
+ viewVersionBuilder.schemaId(newSchema.schemaId());
+ return this;
+ }
+
+ @Override
+ public ViewBuilder withQuery(String dialect, String sql) {
+ representations.add(
+
ImmutableSQLViewRepresentation.builder().dialect(dialect).sql(sql).build());
+ return this;
+ }
+
+ @Override
+ public ViewBuilder withDefaultCatalog(String defaultCatalog) {
+ viewVersionBuilder.defaultCatalog(defaultCatalog);
+ return this;
+ }
+
+ @Override
+ public ViewBuilder withDefaultNamespace(Namespace namespace) {
+ this.defaultNamespace = namespace;
+ return this;
+ }
+
+ @Override
+ public ViewBuilder withProperties(Map<String, String> newProperties) {
+ this.properties.putAll(newProperties);
+ return this;
+ }
+
+ @Override
+ public ViewBuilder withProperty(String key, String value) {
+ this.properties.put(key, value);
+ return this;
+ }
+
+ @Override
+ public View create() {
+ Preconditions.checkState(
+ !representations.isEmpty(), "Cannot create view without specifying a
query");
+ Preconditions.checkState(null != schema, "Cannot create view without
specifying schema");
+ Preconditions.checkState(
+ null != defaultNamespace, "Cannot create view without specifying a
default namespace");
+
+ ViewVersion viewVersion =
+ viewVersionBuilder
+ .versionId(1)
+ .timestampMillis(System.currentTimeMillis())
+ .addAllRepresentations(representations)
+ .defaultNamespace(defaultNamespace)
+ .putSummary("operation", "create")
+ .build();
+
+ ViewMetadata viewMetadata =
+ ViewMetadata.builder()
+ .setProperties(properties)
+ .setLocation("dummy")
+ .setCurrentVersion(viewVersion, schema)
+ .build();
+
+ CreateViewRequest request =
+ ImmutableCreateViewRequest.builder()
+ .metadata(viewMetadata)
+ .name(identifier.name())
+ .build();
+
+ LoadViewResponse response =
+ client.post(
+ paths.views(identifier.namespace()),
+ request,
+ LoadViewResponse.class,
+ headers(context),
+ ErrorHandlers.viewErrorHandler());
+
+ return viewFromResponse(response);
+ }
+
+ @Override
+ public View replace() {
+ Preconditions.checkState(
+ !representations.isEmpty(), "Cannot replace view without specifying
a query");
+ Preconditions.checkState(null != schema, "Cannot replace view without
specifying schema");
+ Preconditions.checkState(
+ null != defaultNamespace, "Cannot replace view without specifying a
default namespace");
+
+ LoadViewResponse response =
+ client.get(
+ paths.view(identifier),
+ LoadViewResponse.class,
+ headers(context),
+ ErrorHandlers.viewErrorHandler());
+
+ ViewMetadata metadata = response.metadata();
+
+ int maxVersionId =
+ metadata.versions().stream()
+ .map(ViewVersion::versionId)
+ .max(Integer::compareTo)
+ .orElseGet(metadata::currentVersionId);
+
+ ViewVersion viewVersion =
+ viewVersionBuilder
+ .versionId(maxVersionId + 1)
+ .timestampMillis(System.currentTimeMillis())
+ .addAllRepresentations(representations)
+ .defaultNamespace(defaultNamespace)
+ .putSummary("operation", "replace")
+ .build();
+
+ ViewMetadata replacement =
+ ViewMetadata.buildFrom(metadata)
+ .setProperties(properties)
+ .setCurrentVersion(viewVersion, schema)
+ .build();
+
+ UpdateTableRequest request =
+ UpdateTableRequest.create(identifier, ImmutableList.of(),
replacement.changes());
+
+ response =
+ client.post(
+ paths.view(identifier),
+ request,
+ LoadViewResponse.class,
+ headers(context),
+ ErrorHandlers.viewErrorHandler());
+
+ return viewFromResponse(response);
+ }
+
+ private BaseView viewFromResponse(LoadViewResponse response) {
+ AuthSession session = tableSession(response.config(), session(context));
+ RESTViewOperations ops =
+ new RESTViewOperations(
+ client,
+ paths.view(identifier),
+ session::headers,
+ tableFileIO(context, response.config()),
+ response.metadata());
+
+ trackFileIO(ops);
+
+ return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
+ }
+
+ @Override
+ public View createOrReplace() {
+ try {
+ client.get(
+ paths.view(identifier),
+ LoadViewResponse.class,
+ headers(context),
+ ErrorHandlers.viewErrorHandler());
+ return replace();
Review Comment:
Like the other builder, this should not load the view twice just to
determine whether to use replace or create. The replace method should be
refactored to accept a LoadViewResponse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
