[#461][MINOR] docs(hive): add doc and IT for Hive column position cha…

…nge (#496) ### What changes were proposed in this pull request? add doc and IT for Hive column position change ### Why are the changes needed? Clearer usage of altering Hive column position Fix: #461 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? IT added
apache · Oct 12, 2023 · 7d87fd3 · 7d87fd3
1 parent 4c601ff
commit 7d87fd3
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 2 deletions.
diff --git a/...atalog-hive/src/main/java/com/datastrato/graviton/catalog/hive/HiveCatalogOperations.java b/...atalog-hive/src/main/java/com/datastrato/graviton/catalog/hive/HiveCatalogOperations.java
@@ -483,7 +483,12 @@ public Table createTable(
   }
 
   /**
-   * Not supported in this implementation. Throws UnsupportedOperationException.
+   * Apply the {@link TableChange change} to an existing Hive table.
+   *
+   * <p>Note: When changing column position, since HMS will check the compatibility of column type
+   * between the old column position and the new column position, you need to make sure that the new
+   * column position is compatible with the old column position, otherwise the operation will fail
+   * in HMS.
    *
    * @param tableIdent The identifier of the table to alter.
    * @param changes The changes to apply to the table.
@@ -563,6 +568,16 @@ public Table alterTable(NameIdentifier tableIdent, TableChange... changes)
       throw new NoSuchTableException(
           String.format("Hive table does not exist: %s in Hive Metastore", tableIdent.name()), e);
     } catch (TException | InterruptedException e) {
+      if (e.getMessage().contains("types incompatible with the existing columns")) {
+        throw new IllegalArgumentException(
+            "Failed to alter Hive table ["
+                + tableIdent.name()
+                + "] in Hive metastore, "
+                + "since Hive metastore will check the compatibility of column type between the old and new column positions, "
+                + "please ensure that the type of the new column position is compatible with the old one, "
+                + "otherwise the alter operation will fail in Hive metastore.",
+            e);
+      }
       throw new RuntimeException(
           "Failed to alter Hive table " + tableIdent.name() + " in Hive metastore", e);
     } catch (IllegalArgumentException e) {

diff --git a/...main/java/com/datastrato/graviton/catalog/lakehouse/iceberg/IcebergCatalogOperations.java b/...main/java/com/datastrato/graviton/catalog/lakehouse/iceberg/IcebergCatalogOperations.java
@@ -346,7 +346,7 @@ public Table loadTable(NameIdentifier tableIdent) throws NoSuchTableException {
   }
 
   /**
-   * Not supported in this implementation. Throws UnsupportedOperationException.
+   * Apply the {@link TableChange change} to an existing Iceberg table.
    *
    * @param tableIdent The identifier of the table to alter.
    * @param changes The changes to apply to the table.

diff --git a/...st/src/test/java/com/datastrato/graviton/integration/test/catalog/hive/CatalogHiveIT.java b/...st/src/test/java/com/datastrato/graviton/integration/test/catalog/hive/CatalogHiveIT.java
@@ -542,6 +542,59 @@ public void testAlterHiveTable() throws TException, InterruptedException {
     Assertions.assertEquals(1, hiveTab.getPartitionKeys().size());
     Assertions.assertEquals(columns[0].name(), hiveTab.getPartitionKeys().get(0).getName());
     assertDefaultTableProperties(alteredTable, hiveTab);
+
+    // test updateColumnPosition exception
+    ColumnDTO col1 =
+        new ColumnDTO.Builder()
+            .withName("name")
+            .withDataType(TypeCreator.NULLABLE.STRING)
+            .withComment("comment")
+            .build();
+    ColumnDTO col2 =
+        new ColumnDTO.Builder()
+            .withName("address")
+            .withDataType(TypeCreator.NULLABLE.STRING)
+            .withComment("comment")
+            .build();
+    ColumnDTO col3 =
+        new ColumnDTO.Builder()
+            .withName("date_of_birth")
+            .withDataType(TypeCreator.NULLABLE.DATE)
+            .withComment("comment")
+            .build();
+    ColumnDTO[] newColumns = new ColumnDTO[] {col1, col2, col3};
+    NameIdentifier tableIdentifier =
+        NameIdentifier.of(
+            metalakeName,
+            catalogName,
+            schemaName,
+            GravitonITUtils.genRandomName("CatalogHiveIT_table"));
+    catalog
+        .asTableCatalog()
+        .createTable(
+            tableIdentifier,
+            newColumns,
+            table_comment,
+            ImmutableMap.of(),
+            new Transform[0],
+            Distribution.NONE,
+            new SortOrder[0]);
+
+    IllegalArgumentException exception =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                catalog
+                    .asTableCatalog()
+                    .alterTable(
+                        tableIdentifier,
+                        TableChange.updateColumnPosition(
+                            new String[] {"date_of_birth"}, TableChange.ColumnPosition.first())));
+    Assertions.assertTrue(
+        exception
+            .getMessage()
+            .contains(
+                "please ensure that the type of the new column position is compatible with the old one"));
   }
 
   private void assertDefaultTableProperties(