amogh-jahagirdar commented on code in PR #7913:
URL: https://github.com/apache/iceberg/pull/7913#discussion_r1347539389


##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -374,4 +385,107 @@ static TableMetadata commit(TableOperations ops, 
UpdateTableRequest request) {
 
     return ops.current();
   }
+
+  public static BaseView baseView(View view) {
+    Preconditions.checkArgument(view instanceof BaseView, "View must be a 
BaseView");
+    return (BaseView) view;
+  }
+
+  public static ListTablesResponse listViews(ViewCatalog catalog, Namespace 
namespace) {
+    return 
ListTablesResponse.builder().addAll(catalog.listViews(namespace)).build();
+  }
+
+  public static LoadViewResponse createView(
+      ViewCatalog catalog, Namespace namespace, CreateViewRequest request) {
+    request.validate();
+
+    ViewMetadata viewMetadata = request.metadata();
+    ViewBuilder viewBuilder =
+        catalog
+            .buildView(TableIdentifier.of(namespace, request.name()))
+            .withSchema(viewMetadata.schema())
+            .withProperties(viewMetadata.properties())
+            
.withDefaultNamespace(viewMetadata.currentVersion().defaultNamespace())
+            
.withDefaultCatalog(viewMetadata.currentVersion().defaultCatalog());
+    viewMetadata.currentVersion().representations().stream()
+        .filter(r -> r instanceof SQLViewRepresentation)
+        .map(r -> (SQLViewRepresentation) r)
+        .forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql()));
+    View view = viewBuilder.create();
+
+    return viewResponse(view);
+  }
+
+  private static LoadViewResponse viewResponse(View view) {
+    ViewMetadata metadata = baseView(view).operations().current();
+    return ImmutableLoadViewResponse.builder()
+        .metadata(metadata)
+        .metadataLocation(metadata.metadataFileLocation())
+        .build();
+  }
+
+  public static LoadViewResponse loadView(ViewCatalog catalog, TableIdentifier 
viewIdentifier) {
+    View view = catalog.loadView(viewIdentifier);
+    return viewResponse(view);
+  }
+
+  public static LoadViewResponse updateView(
+      ViewCatalog catalog, TableIdentifier ident, UpdateTableRequest request) {
+    View view = catalog.loadView(ident);
+    ViewMetadata metadata = commit(baseView(view).operations(), request);
+
+    return ImmutableLoadViewResponse.builder()
+        .metadata(metadata)
+        .metadataLocation(metadata.metadataFileLocation())
+        .build();
+  }
+
+  public static void renameView(ViewCatalog catalog, RenameTableRequest 
request) {
+    catalog.renameView(request.source(), request.destination());
+  }
+
+  public static void dropView(ViewCatalog catalog, TableIdentifier 
viewIdentifier) {
+    boolean dropped = catalog.dropView(viewIdentifier);
+    if (!dropped) {
+      throw new NoSuchViewException("View does not exist: %s", viewIdentifier);
+    }
+  }
+
+  static ViewMetadata commit(ViewOperations ops, UpdateTableRequest request) {
+    AtomicBoolean isRetry = new AtomicBoolean(false);
+    try {
+      Tasks.foreach(ops)
+          .retry(COMMIT_NUM_RETRIES_DEFAULT)
+          .exponentialBackoff(
+              COMMIT_MIN_RETRY_WAIT_MS_DEFAULT,
+              COMMIT_MAX_RETRY_WAIT_MS_DEFAULT,
+              COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT,
+              2.0 /* exponential */)
+          .onlyRetryOn(CommitFailedException.class)
+          .run(
+              taskOps -> {
+                ViewMetadata base = isRetry.get() ? taskOps.refresh() : 
taskOps.current();
+                isRetry.set(true);
+
+                // apply changes
+                ViewMetadata.Builder metadataBuilder = 
ViewMetadata.buildFrom(base);
+                request.updates().forEach(update -> 
update.applyTo(metadataBuilder));

Review Comment:
   I see we're currently running through the validations on line 486 but if I'm 
not mistaken right now they're all just no-ops, is that right @nastra ? 
   
   Few requirements that come to mind:
   
   1.) For creating a view, there probably needs to be a requirement similar to 
table's `AssertTableDoesNotExist` but for views, `AssertViewDoesNotExist`. 
Similar principle for ReplaceView and verifying the UUID as expected.
   
   
   2.) I don't see a metadata update for AddViewRepresentation but I'd imagine 
that should validate that the SQL dialect does not already exist for the 
specified version. I forgot where we landed on that discussion, We do want to 
prevent that right?
   
   3.) A requirement for the schema ID to make sure that is unchanged from the 
base 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