nastra commented on code in PR #173:
URL: https://github.com/apache/iceberg-go/pull/173#discussion_r1812088531


##########
catalog/glue.go:
##########
@@ -122,38 +165,104 @@ func (c *GlueCatalog) LoadTable(ctx context.Context, 
identifier table.Identifier
        return icebergTable, nil
 }
 
-func (c *GlueCatalog) CatalogType() CatalogType {
-       return Glue
-}
-
+// DropTable deletes an Iceberg table from the Glue catalog.
 func (c *GlueCatalog) DropTable(ctx context.Context, identifier 
table.Identifier) error {
-       return fmt.Errorf("%w: [Glue Catalog] drop table", 
iceberg.ErrNotImplemented)
+       database, tableName, err := identifierToGlueTable(identifier)
+       if err != nil {
+               return err
+       }
+
+       // Check if the table exists and is an Iceberg table.
+       _, err = c.getTable(ctx, database, tableName)
+       if err != nil {
+               return err
+       }
+
+       params := &glue.DeleteTableInput{
+               CatalogId:    c.catalogId,
+               DatabaseName: aws.String(database),
+               Name:         aws.String(tableName),
+       }
+       _, err = c.glueSvc.DeleteTable(ctx, params)
+       if err != nil {
+               return fmt.Errorf("failed to drop table %s.%s: %w", database, 
tableName, err)
+       }
+
+       return nil
 }
 
+// RenameTable renames an Iceberg table in the Glue catalog.
 func (c *GlueCatalog) RenameTable(ctx context.Context, from, to 
table.Identifier) (*table.Table, error) {
-       return nil, fmt.Errorf("%w: [Glue Catalog] rename table", 
iceberg.ErrNotImplemented)
-}
+       fromDatabase, fromTable, err := identifierToGlueTable(from)
+       if err != nil {
+               return nil, err
+       }
 
-func (c *GlueCatalog) CreateNamespace(ctx context.Context, namespace 
table.Identifier, props iceberg.Properties) error {
-       return fmt.Errorf("%w: [Glue Catalog] create namespace", 
iceberg.ErrNotImplemented)
-}
+       toDatabase, toTable, err := identifierToGlueTable(to)
+       if err != nil {
+               return nil, err
+       }
 
-func (c *GlueCatalog) DropNamespace(ctx context.Context, namespace 
table.Identifier) error {

Review Comment:
   moving methods around makes this quite difficult to review. I'd suggest to 
just add table-related methods after the namespace-related methods



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