rdblue commented on code in PR #7913:
URL: https://github.com/apache/iceberg/pull/7913#discussion_r1367565262


##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -971,4 +996,261 @@ 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 Map<String, String> properties = Maps.newHashMap();
+    private final List<ViewRepresentation> representations = 
Lists.newArrayList();
+    private Namespace defaultNamespace = null;
+    private String defaultCatalog = null;
+    private Schema schema = null;
+    private String location = null;
+
+    private RESTViewBuilder(SessionContext context, TableIdentifier 
identifier) {
+      checkViewIdentifierIsValid(identifier);
+      this.identifier = identifier;
+      this.context = context;
+    }
+
+    @Override
+    public ViewBuilder withSchema(Schema newSchema) {
+      this.schema = newSchema;
+      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 catalog) {
+      this.defaultCatalog = catalog;
+      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 ViewBuilder withLocation(String newLocation) {
+      this.location = newLocation;
+      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 =
+          ImmutableViewVersion.builder()
+              .versionId(1)
+              .schemaId(schema.schemaId())
+              .addAllRepresentations(representations)
+              .defaultNamespace(defaultNamespace)
+              .defaultCatalog(defaultCatalog)
+              .timestampMillis(System.currentTimeMillis())
+              .putSummary("operation", "create")
+              .build();
+
+      CreateViewRequest request =
+          ImmutableCreateViewRequest.builder()
+              .name(identifier.name())
+              .location(location)
+              .schema(schema)
+              .viewVersion(viewVersion)
+              .properties(properties)
+              .build();
+
+      LoadViewResponse response =
+          client.post(
+              paths.views(identifier.namespace()),
+              request,
+              LoadViewResponse.class,
+              headers(context),
+              ErrorHandlers.viewErrorHandler());
+
+      return viewFromResponse(response);
+    }
+
+    @Override
+    public View createOrReplace() {
+      try {
+        return replace(loadView());
+      } catch (NoSuchViewException e) {
+        return create();
+      }
+    }
+
+    @Override
+    public View replace() {
+      return replace(loadView());
+    }
+
+    private LoadViewResponse loadView() {
+      return client.get(
+          paths.view(identifier),
+          LoadViewResponse.class,
+          headers(context),
+          ErrorHandlers.viewErrorHandler());
+    }
+
+    private View replace(LoadViewResponse response) {
+      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");
+
+      ViewMetadata metadata = response.metadata();
+
+      int maxVersionId =
+          metadata.versions().stream()
+              .map(ViewVersion::versionId)
+              .max(Integer::compareTo)
+              .orElseGet(metadata::currentVersionId);
+
+      ViewVersion viewVersion =
+          ImmutableViewVersion.builder()
+              .versionId(maxVersionId + 1)
+              .schemaId(schema.schemaId())
+              .addAllRepresentations(representations)
+              .defaultNamespace(defaultNamespace)
+              .defaultCatalog(defaultCatalog)
+              .timestampMillis(System.currentTimeMillis())
+              .putSummary("operation", "replace")
+              .build();
+
+      ViewMetadata.Builder builder =
+          ViewMetadata.buildFrom(metadata)
+              .setProperties(properties)
+              .setCurrentVersion(viewVersion, schema);
+
+      if (null != location) {
+        builder.setLocation(location);
+      }
+
+      ViewMetadata replacement = builder.build();
+
+      UpdateTableRequest request =
+          UpdateTableRequest.create(identifier, ImmutableList.of(), 
replacement.changes());

Review Comment:
   I don't think this should create its own request and call the route 
directly. This duplicates logic that is in `RESTViewOperations`. Instead, this 
should create `RESTViewOperations` initially and then use it to commit the 
replacement metadata.



-- 
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

Reply via email to