Merge pull request #589 from open-metadata/issue568

Fix #568 Add tests for APIs related to table complex column type

catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java

@@ -342,6 +342,10 @@ public abstract class TableRepository {
   }
 
   private void validateTags(List<Column> columns) {
+    if (columns == null || columns.isEmpty()) {
+      return;
+    }
+
     columns.forEach(column -> {
       EntityUtil.validateTags(tagDAO(), column.getTags());
       if (column.getChildren() != null) {
@@ -392,6 +396,9 @@ public abstract class TableRepository {
   }
 
   List<Column> cloneWithoutTags(List<Column> columns) {
+    if (columns == null || columns.isEmpty()) {
+      return columns;
+    }
     List<Column> copy = new ArrayList<>();
     columns.forEach(c -> copy.add(cloneWithoutTags(c)));
     return copy;
@@ -402,9 +409,9 @@ public abstract class TableRepository {
     return new Column().withDescription(column.getDescription()).withName(column.getName())
             .withFullyQualifiedName(column.getFullyQualifiedName())
             .withArrayDataType(column.getArrayDataType())
-            .withColumnConstraint(column.getColumnConstraint())
+            .withConstraint(column.getConstraint())
-            .withColumnDataTypeDisplay(column.getColumnDataTypeDisplay())
+            .withDataTypeDisplay(column.getDataTypeDisplay())
-            .withColumnDataType(column.getColumnDataType())
+            .withDataType(column.getDataType())
             .withDataLength(column.getDataLength())
             .withOrdinalPosition(column.getOrdinalPosition())
             .withChildren(children);
@@ -453,19 +460,10 @@ public abstract class TableRepository {
    * Update the backend database
    */
   private void patch(Table original, Table updated) throws IOException {
-    // TODO Patching field usageSummary is ignored
+    // Patch can't make changes to following fields. Ignore the change
-    if (!original.getId().equals(updated.getId())) {
+    updated.withFullyQualifiedName(original.getFullyQualifiedName()).withName(original.getName())
-      throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute(Entity.TABLE, "id"));
+            .withDatabase(original.getDatabase()).withId(original.getId());
-    }
+
-    if (!original.getName().equals(updated.getName())) {
-      throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute(Entity.TABLE, "name"));
-    }
-    if (updated.getDatabase() == null) {
-      throw new IllegalArgumentException("Table relationship database can't be removed");
-    }
-    if (!updated.getDatabase().getId().equals(original.getDatabase().getId())) {
-      throw new IllegalArgumentException("Table relationship database can't be replaced");
-    }
     validateRelationships(updated, updated.getDatabase().getId(), updated.getOwner());
 
     // Remove previous tags. Merge tags from the update and the existing tags
@@ -524,7 +522,8 @@ public abstract class TableRepository {
       // Find stored column matching name, data type and ordinal position
       Column stored = storedColumns.stream()
               .filter(s -> s.getName().equals(updated.getName()) &&
-                      s.getColumnDataType() == updated.getColumnDataType() &&
+                      s.getDataType() == updated.getDataType() &&
+                      s.getArrayDataType() == updated.getArrayDataType() &&
                       Objects.equals(s.getOrdinalPosition(), updated.getOrdinalPosition()))
               .findAny()
               .orElse(null);
@@ -551,11 +550,13 @@ public abstract class TableRepository {
       }
     }
 
+
     for (Column stored : storedColumns) {
       // Find updated column matching name, data type and ordinal position
-      Column  updated = storedColumns.stream()
+      Column  updated = updatedColumns.stream()
               .filter(s -> s.getName().equals(stored.getName()) &&
-                      s.getColumnDataType() == stored.getColumnDataType() &&
+                      s.getDataType() == stored.getDataType() &&
+                      s.getArrayDataType() == stored.getArrayDataType() &&
                       Objects.equals(s.getOrdinalPosition(), stored.getOrdinalPosition()))
               .findAny()
               .orElse(null);

catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/DatabaseUtil.java

@@ -16,13 +16,16 @@
 
 package org.openmetadata.catalog.resources.databases;
 
+import org.openmetadata.catalog.entity.data.Table;
 import org.openmetadata.catalog.type.Column;
 import org.openmetadata.catalog.type.ColumnConstraint;
+import org.openmetadata.catalog.type.ColumnDataType;
 import org.openmetadata.catalog.type.TableConstraint;
 import org.openmetadata.catalog.type.TableType;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 
 public final class DatabaseUtil {
   private DatabaseUtil() {
@@ -32,7 +35,7 @@ public final class DatabaseUtil {
   public static boolean validateSinglePrimaryColumn(List<Column> columns) {
     int primaryKeyColumns = 0;
     for (Column c : columns) {
-      if (c.getColumnConstraint() == ColumnConstraint.PRIMARY_KEY) {
+      if (c.getConstraint() == ColumnConstraint.PRIMARY_KEY) {
         primaryKeyColumns++;
         if (primaryKeyColumns > 1) {
           throw new IllegalArgumentException("Multiple columns tagged with primary key constraints");
@@ -68,10 +71,82 @@ public final class DatabaseUtil {
   }
 
   public static void validateViewDefinition(TableType tableType, String viewDefinition) {
-    if ( (tableType == null || tableType.equals(TableType.Regular) || tableType.equals(TableType.External))
+    if ((tableType == null || tableType.equals(TableType.Regular) || tableType.equals(TableType.External))
             && viewDefinition != null && !viewDefinition.isEmpty()) {
       throw new IllegalArgumentException("ViewDefinition can only be set on TableType View, " +
               "SecureView or MaterializedView");
     }
   }
+
+  public static void validateColumns(Table table) {
+    for (Column c : table.getColumns()) {
+      validateColumnDataTypeDisplay(c);
+      validateColumnDataLength(c);
+      validateArrayColumn(c);
+      validateStructColumn(c);
+    }
+  }
+
+  public static void validateColumnDataTypeDisplay(Column column) {
+    // If dataTypeDisplay is null then set it based on dataType
+    if (column.getDataTypeDisplay() == null) {
+      column.setDataTypeDisplay(column.getDataType().value().toLowerCase(Locale.ROOT));
+    }
+
+    // Make sure types from column dataType and dataTypeDisplay match
+    String dataType = column.getDataType().value().toLowerCase(Locale.ROOT);
+    String dataTypeDisplay = column.getDataTypeDisplay().toLowerCase(Locale.ROOT);
+
+    if (!dataTypeDisplay.startsWith(dataType)) {
+      throw new IllegalArgumentException(String.format("columnDataType %s does not match columnDataTypeDisplay %s",
+              dataType, dataTypeDisplay));
+    }
+
+    column.setDataTypeDisplay(dataTypeDisplay); // Make dataTypeDisplay lower case
+  }
+
+  public static void validateColumnDataLength(Column column) {
+    // Types char, varchar, binary, varbinary must have dataLength
+    ColumnDataType dataType = column.getDataType();
+    if ((dataType == ColumnDataType.CHAR || dataType == ColumnDataType.VARCHAR ||
+            dataType == ColumnDataType.BINARY || dataType == ColumnDataType.VARBINARY) && column.getDataLength() == null) {
+      throw new IllegalArgumentException("For column data types char, varchar, binary, varbinary dataLength " +
+              "must not be null");
+    }
+  }
+
+  public static void validateArrayColumn(Column column) {
+    // arrayDataType must only be used when columnDataType is array. Ignore the arrayDataType.
+    ColumnDataType dataType = column.getDataType();
+    if (column.getArrayDataType() != null && dataType != ColumnDataType.ARRAY) {
+      column.setArrayDataType(null);
+    }
+
+    if (dataType == ColumnDataType.ARRAY) {
+      if (column.getArrayDataType() == null) {
+        throw new IllegalArgumentException("For column data type array, arrayDataType " +
+                "must not be null");
+      }
+
+      if (!column.getDataTypeDisplay().startsWith("array<")) {
+        throw new IllegalArgumentException("For column data type array, dataTypeDisplay must be of type " +
+                "array<arrayDataType>");
+      }
+    }
+  }
+
+  public static void validateStructColumn(Column column) {
+    ColumnDataType dataType = column.getDataType();
+    if (dataType == ColumnDataType.STRUCT) {
+      if (column.getChildren() == null) {
+        throw new IllegalArgumentException("For column data type struct, children " +
+                "must not be null");
+      }
+
+      if (!column.getDataTypeDisplay().startsWith("struct<")) {
+        throw new IllegalArgumentException("For column data type struct, dataTypeDisplay must be of type " +
+                "stuct<member fields>");
+      }
+    }
+  }
 }

catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java

@@ -17,8 +17,6 @@
 package org.openmetadata.catalog.resources.databases;
 
 import com.google.inject.Inject;
-import org.openmetadata.catalog.type.TableData;
-import org.openmetadata.catalog.type.TableJoins;
 import io.swagger.annotations.Api;
 import io.swagger.v3.oas.annotations.ExternalDocumentation;
 import io.swagger.v3.oas.annotations.Operation;
@@ -34,7 +32,11 @@ import org.openmetadata.catalog.jdbi3.TableRepository;
 import org.openmetadata.catalog.resources.Collection;
 import org.openmetadata.catalog.security.CatalogAuthorizer;
 import org.openmetadata.catalog.security.SecurityUtil;
+import org.openmetadata.catalog.type.Column;
+import org.openmetadata.catalog.type.ColumnDataType;
 import org.openmetadata.catalog.type.EntityReference;
+import org.openmetadata.catalog.type.TableData;
+import org.openmetadata.catalog.type.TableJoins;
 import org.openmetadata.catalog.type.TableProfile;
 import org.openmetadata.catalog.util.EntityUtil;
 import org.openmetadata.catalog.util.EntityUtil.Fields;
@@ -71,6 +73,7 @@ import java.security.GeneralSecurityException;
 import java.text.ParseException;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.UUID;
 
@@ -387,6 +390,7 @@ public class TableResource {
     table.setId(UUID.randomUUID());
     DatabaseUtil.validateConstraints(table.getColumns(), table.getTableConstraints());
     DatabaseUtil.validateViewDefinition(table.getTableType(), table.getViewDefinition());
+    DatabaseUtil.validateColumns(table);
     return table;
   }
 }

catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java

@@ -74,7 +74,12 @@ public final class JsonUtils {
   }
 
   public static String pojoToJson(Object o) throws JsonProcessingException {
-    return OBJECT_MAPPER.writeValueAsString(o);
+    return pojoToJson(o, false);
+  }
+
+  public static String pojoToJson(Object o, boolean prettyPrint) throws JsonProcessingException {
+    return prettyPrint ? OBJECT_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(o) :
+            OBJECT_MAPPER.writeValueAsString(o);
   }
 
   public static JsonStructure getJsonStructure(Object o) {

catalog-rest-service/src/main/resources/json/schema/entity/data/table.json

@@ -35,7 +35,7 @@
         }
       ]
     },
-    "columnDataType": {
+    "dataType": {
       "javaType": "org.openmetadata.catalog.type.ColumnDataType",
       "description": "This enum defines the type of data stored in a column.",
       "type": "string",
@@ -76,7 +76,7 @@
         "JSON"
       ]
     },
-    "columnConstraint": {
+    "constraint": {
       "javaType": "org.openmetadata.catalog.type.ColumnConstraint",
       "description": "This enum defines the type for column constraint.",
       "type": "string",
@@ -111,7 +111,7 @@
       }
     },
     "columnName": {
-      "description": "Local name (not fully qualified name) of the column. ColumnName is `-` when the column is not named in struct columnDataType. For example, BigQuery supports struct with unnamed fields",
+      "description": "Local name (not fully qualified name) of the column. ColumnName is `-` when the column is not named in struct dataType. For example, BigQuery supports struct with unnamed fields",
       "type": "string",
       "minLength": 1,
       "maxLength": 64,
@@ -125,7 +125,7 @@
       "pattern": "^[^.]*$"
     },
     "fullyQualifiedColumnName": {
-      "description": "Fully qualified name of the column that includes `serviceName.databaseName.tableName.columnName[.nestedColumnName]`. When columnName is null for columnDataType struct fields, `field_#` where `#` is field index is used. For map columnDataType, for key the field name `key` is used and for the value field `value` is used.",
+      "description": "Fully qualified name of the column that includes `serviceName.databaseName.tableName.columnName[.nestedColumnName]`. When columnName is null for dataType struct fields, `field_#` where `#` is field index is used. For map dataType, for key the field name `key` is used and for the value field `value` is used.",
       "type": "string",
       "minLength": 1,
       "maxLength": 256
@@ -138,19 +138,21 @@
         "name": {
           "$ref": "#/definitions/columnName"
         },
-        "columnDataType": {
+        "dataType": {
           "description": "Data type of the column (int, date etc.).",
-          "$ref": "#/definitions/columnDataType"
+          "$ref": "#/definitions/dataType"
         },
         "arrayDataType" : {
-          "description": "Data type used array in columnDataType. For example, `array<int>` has columnDataType as `array` and arrayDataType as `int`."
+          "description": "Data type used array in dataType. For example, `array<int>` has dataType as `array` and arrayDataType as `int`.",
+          "$ref": "#/definitions/dataType"
         },
         "dataLength" : {
-          "description": "Length of `char`, `varchar`, `binary`, `varbinary` `columnDataTypes`, else null. For example, `varchar(20)` has columnDataType as `varchar` and dataLength as `20`.",
+          "description": "Length of `char`, `varchar`, `binary`, `varbinary` `dataTypes`, else null. For example, `varchar(20)` has dataType as `varchar` and dataLength as `20`.",
           "type": "integer"
         },
-        "columnDataTypeDisplay" : {
+        "dataTypeDisplay" : {
-          "description" : "Display name used for columnDataType. This is useful for complex types, such as `array<int>, map<int,string>, struct<>, and union types."
+          "description" : "Display name used for dataType. This is useful for complex types, such as `array<int>, map<int,string>, struct<>, and union types.",
+          "type": "string"
         },
         "description": {
           "description": "Description of the column.",
@@ -167,29 +169,30 @@
           },
           "default": null
         },
-        "columnConstraint": {
+        "constraint": {
           "description": "Column level constraint.",
-          "$ref": "#/definitions/columnConstraint"
+          "$ref": "#/definitions/constraint"
         },
         "ordinalPosition": {
           "description": "Ordinal position of the column.",
           "type": "integer"
         },
         "jsonSchema" : {
-          "description": "Json schema only if the columnDataType is JSON else null.",
+          "description": "Json schema only if the dataType is JSON else null.",
           "type": "string"
         },
         "children" : {
-          "description": "Child columns if columnDataType or arrayDataType is `map`, `struct`, or `union` else `null`.",
+          "description": "Child columns if dataType or arrayDataType is `map`, `struct`, or `union` else `null`.",
           "type": "array",
           "items": {
             "$ref": "#/definitions/column"
-          }
+          },
+          "default" : null
         }
       },
       "required": [
         "name",
-        "columnDataType"
+        "dataType"
       ],
       "additionalProperties": false
     },

catalog-rest-service/src/main/resources/json/schema/type/tagLabel.json

@@ -34,5 +34,6 @@
       "description": "Link to the tag resource.",
       "$ref": "basic.json#/definitions/href"
     }
-  }
+  },
+  "additionalProperties": false
 }

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java

@@ -72,6 +72,7 @@ import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.UUID;
 import java.util.stream.Collectors;
@@ -91,6 +92,13 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.openmetadata.catalog.resources.databases.DatabaseResourceTest.createAndCheckDatabase;
 import static org.openmetadata.catalog.resources.services.DatabaseServiceResourceTest.createService;
+import static org.openmetadata.catalog.type.ColumnDataType.ARRAY;
+import static org.openmetadata.catalog.type.ColumnDataType.BIGINT;
+import static org.openmetadata.catalog.type.ColumnDataType.CHAR;
+import static org.openmetadata.catalog.type.ColumnDataType.FLOAT;
+import static org.openmetadata.catalog.type.ColumnDataType.INT;
+import static org.openmetadata.catalog.type.ColumnDataType.STRING;
+import static org.openmetadata.catalog.type.ColumnDataType.STRUCT;
 import static org.openmetadata.catalog.util.RestUtil.DATE_FORMAT;
 import static org.openmetadata.catalog.util.TestUtils.NON_EXISTENT_ENTITY;
 import static org.openmetadata.catalog.util.TestUtils.adminAuthHeaders;
@@ -110,12 +118,9 @@ public class TableResourceTest extends CatalogApplicationTest {
   public static final TagLabel TIER_2 = new TagLabel().withTagFQN("Tier.Tier2");
 
   public static List<Column> COLUMNS = Arrays.asList(
-          new Column().withName("c1").withColumnDataType(ColumnDataType.BIGINT)
+          getColumn("c1", BIGINT, USER_ADDRESS_TAG_LABEL),
-                  .withTags(singletonList(USER_ADDRESS_TAG_LABEL)),
+          getColumn("c2", ColumnDataType.VARCHAR, USER_ADDRESS_TAG_LABEL).withDataLength(10),
-          new Column().withName("c2").withColumnDataType(ColumnDataType.BIGINT)
+          getColumn("c3", BIGINT, USER_BANK_ACCOUNT_TAG_LABEL));
-                  .withTags(singletonList(USER_ADDRESS_TAG_LABEL)),
-          new Column().withName("c3").withColumnDataType(ColumnDataType.BIGINT)
-                  .withTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)));
 
   public static User USER1;
   public static EntityReference USER_OWNER1;
@@ -160,6 +165,41 @@ public class TableResourceTest extends CatalogApplicationTest {
     assertResponse(exception, BAD_REQUEST, "[name size must be between 1 and 64]");
   }
 
+  @Test
+  public void post_tableWithoutColumnDataLength_400(TestInfo test) {
+    List<Column> columns = singletonList(getColumn("c1", BIGINT, null).withOrdinalPosition(1));
+    CreateTable create = create(test).withColumns(columns);
+
+    // char, varchar, binary, and varbinary columns must have length
+    ColumnDataType[] columnDataTypes = {CHAR, ColumnDataType.VARCHAR, ColumnDataType.BINARY,
+            ColumnDataType.VARBINARY};
+
+    for (ColumnDataType dataType : columnDataTypes) {
+      create.getColumns().get(0).withDataType(dataType);
+      HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
+              createTable(create, adminAuthHeaders()));
+      assertResponse(exception, BAD_REQUEST,
+              "For column data types char, varchar, binary, varbinary dataLength must not be null");
+    }
+  }
+
+  @Test
+  public void post_tableInvalidArrayColumn_400(TestInfo test) {
+    // No arrayDataType passed for array
+    List<Column> columns = singletonList(getColumn("c1", ARRAY, "array<int>", null));
+    CreateTable create = create(test).withColumns(columns);
+    HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
+            createTable(create, adminAuthHeaders()));
+    assertResponse(exception, BAD_REQUEST, "For column data type array, arrayDataType must not be null");
+
+    // No dataTypeDisplay passed for array
+    columns.get(0).withArrayDataType(INT).withDataTypeDisplay(null);
+    exception = assertThrows(HttpResponseException.class, () ->
+            createTable(create, adminAuthHeaders()));
+    assertResponse(exception, BAD_REQUEST,
+            "For column data type array, dataTypeDisplay must be of type array<arrayDataType>");
+  }
+
   @Test
   public void post_tableAlreadyExists_409_conflict(TestInfo test) throws HttpResponseException {
     CreateTable create = create(test);
@@ -181,6 +221,85 @@ public class TableResourceTest extends CatalogApplicationTest {
     createAndCheckTable(create, adminAuthHeaders());
   }
 
+  private static Column getColumn(String name, ColumnDataType columnDataType, TagLabel tag) {
+
+    return getColumn(name, columnDataType, null, tag);
+  }
+
+  private static Column getColumn(String name, ColumnDataType columnDataType, String dataTypeDisplay, TagLabel tag) {
+
+    return new Column().withName(name).withDataType(columnDataType)
+            .withDataTypeDisplay(dataTypeDisplay).withTags(singletonList(tag));
+  }
+
+  @Test
+  public void post_put_patch_complexColumnTypes(TestInfo test) throws HttpResponseException, JsonProcessingException {
+    Column c1 = getColumn("c1", ARRAY, "array<int>", USER_ADDRESS_TAG_LABEL).withArrayDataType(INT);
+    Column c2_a = getColumn("a", INT, USER_ADDRESS_TAG_LABEL);
+    Column c2_b = getColumn("b", CHAR, USER_ADDRESS_TAG_LABEL);
+    Column c2_c_d = getColumn("d", INT, USER_ADDRESS_TAG_LABEL);
+    Column c2_c = getColumn("c", STRUCT, "struct<int: d>>>", USER_ADDRESS_TAG_LABEL)
+            .withChildren(new ArrayList<>(Arrays.asList(c2_c_d)));
+
+    // Column struct<a: int, b:char, c: struct<int: d>>>
+    Column c2 = getColumn("c2", STRUCT, "struct<a: int, b:string, c: struct<int: d>>>",USER_BANK_ACCOUNT_TAG_LABEL)
+            .withChildren(new ArrayList<>(Arrays.asList(c2_a, c2_b, c2_c)));
+
+    // Test POST operation can create complex types
+    CreateTable create1 = create(test, 1).withColumns(Arrays.asList(c1, c2));
+    Table table1 = createAndCheckTable(create1, adminAuthHeaders());
+
+    // Test PUT operation
+    CreateTable create2 = create(test, 2).withColumns(Arrays.asList(c1, c2));
+    updateAndCheckTable(create2.withName("put_complexColumnType"), Status.CREATED, adminAuthHeaders());
+    // Update without any change
+    updateAndCheckTable(create2.withName("put_complexColumnType"), Status.OK, adminAuthHeaders());
+
+    //
+    // Update the complex columns
+    //
+    // c1 from array<int> to array<char> and also change the tag
+    c1.withArrayDataType(CHAR).withTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)).withDataTypeDisplay("array<char>");
+
+    // c2 from -> to
+    // struct<a:int, b:char, c:struct<d:int>>>
+    // struct<-----, b:char, c:struct<d:int, e:char>, f:char>
+    c2_b.withTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)); // Change c2.b tag
+    c2_c.getChildren().add(getColumn("e", INT,USER_ADDRESS_TAG_LABEL)); // Add c2.c.e
+    c2.getChildren().remove(0); // Remove c2.a from struct
+    c2.getChildren().add(getColumn("f", CHAR, USER_ADDRESS_TAG_LABEL)); // Add c2.f
+    create2 = create2.withColumns(Arrays.asList(c1, c2));
+
+    // Update the columns with put operation and validate update
+    updateAndCheckTable(create2.withName("put_complexColumnType"), Status.OK, adminAuthHeaders());
+
+
+    //
+    // Patch operations on table1 created by POST operation. Columns can't be added or deleted. Only tags and
+    // description can be changed
+    //
+    String tableJson = JsonUtils.pojoToJson(table1);
+    c1 = table1.getColumns().get(0);
+    c1.withTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)); // c1 tag changed
+
+    c2 = table1.getColumns().get(1);
+    c2.withTags(Arrays.asList(USER_ADDRESS_TAG_LABEL, USER_BANK_ACCOUNT_TAG_LABEL)); // c2 new tag added
+
+    c2_a = c2.getChildren().get(0);
+    c2_a.withTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)); // c2.a tag changed
+
+    c2_b = c2.getChildren().get(1);
+    c2_b.withTags(new ArrayList<>()); // c2.b tag removed
+
+    c2_c = c2.getChildren().get(2);
+    c2_c.withTags(new ArrayList<>()); // c2.c tag removed
+
+    c2_c_d = c2_c.getChildren().get(0);
+    c2_c_d.setTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)); // c2.c.d new tag added
+    table1 = patchTable(tableJson, table1, adminAuthHeaders());
+    validateColumns(Arrays.asList(c1, c2), table1.getColumns());
+  }
+
   @Test
   public void post_tableWithUserOwner_200_ok(TestInfo test) throws HttpResponseException {
     createAndCheckTable(create(test).withOwner(USER_OWNER1), adminAuthHeaders());
@@ -313,8 +432,9 @@ public class TableResourceTest extends CatalogApplicationTest {
     // Create a table with column c1, type BIGINT, description c1 and tag USER_ADDRESS_TAB_LABEL
     //
     List<TagLabel> tags = new ArrayList<>(singletonList(USER_ADDRESS_TAG_LABEL));
-    List<Column> columns = singletonList(new Column().withName("c1").withColumnDataType(ColumnDataType.BIGINT)
+    List<Column> columns = singletonList(new Column().withName("c1").withDataType(BIGINT)
             .withOrdinalPosition(1).withDescription("c1").withTags(tags));
+
     CreateTable request = create(test).withColumns(columns);
     Table table = createAndCheckTable(request, adminAuthHeaders());
     columns.get(0).setFullyQualifiedName(table.getFullyQualifiedName() + ".c1");
@@ -331,7 +451,7 @@ public class TableResourceTest extends CatalogApplicationTest {
     //
     tags.add(USER_BANK_ACCOUNT_TAG_LABEL);
     List<Column> updatedColumns = new ArrayList<>();
-    updatedColumns.add(new Column().withName("c1").withColumnDataType(ColumnDataType.BIGINT)
+    updatedColumns.add(new Column().withName("c1").withDataType(BIGINT)
             .withTags(tags).withOrdinalPosition(1));
     table = updateAndCheckTable(request.withColumns(updatedColumns), OK, adminAuthHeaders());
 
@@ -343,8 +463,8 @@ public class TableResourceTest extends CatalogApplicationTest {
     //
     // Add a new column and make sure it is added by PUT
     //
-    updatedColumns.add(new Column().withName("c2").withColumnDataType(ColumnDataType.BINARY).withOrdinalPosition(2)
+    updatedColumns.add(new Column().withName("c2").withDataType(ColumnDataType.BINARY).withOrdinalPosition(2)
-            .withFullyQualifiedName(table.getFullyQualifiedName() + ".c2").withTags(tags));
+            .withDataLength(10).withFullyQualifiedName(table.getFullyQualifiedName() + ".c2").withTags(tags));
     table = updateAndCheckTable(request.withColumns(updatedColumns), OK, adminAuthHeaders());
     assertEquals(2, table.getColumns().size());
     validateTags(updatedColumns.get(0).getTags(), table.getColumns().get(0).getTags());
@@ -515,6 +635,20 @@ public class TableResourceTest extends CatalogApplicationTest {
     assertEquals(expected, actual.getColumnJoins());
   }
 
+  public static class TagLabelComparator implements Comparator<TagLabel> {
+    @Override
+    public int compare(TagLabel label, TagLabel t1) {
+      return label.getTagFQN().compareTo(t1.getTagFQN());
+    }
+  }
+
+  public static class ColumnComparator implements Comparator<Column> {
+    @Override
+    public int compare(Column column, Column t1) {
+      return column.getName().compareTo(t1.getName());
+    }
+  }
+
   public static class ColumnJoinComparator implements Comparator<ColumnJoin> {
     @Override
     public int compare(ColumnJoin columnJoin, ColumnJoin t1) {
@@ -596,7 +730,7 @@ public class TableResourceTest extends CatalogApplicationTest {
   }
 
   @Test
-  public void put_viewDefinition_invalid_table_4xx(TestInfo test) throws HttpResponseException {
+  public void put_viewDefinition_invalid_table_4xx(TestInfo test) {
     CreateTable createTable = create(test);
     createTable.setTableType(TableType.Regular);
     String query = "sales_vw\n" +
@@ -615,7 +749,6 @@ public class TableResourceTest extends CatalogApplicationTest {
   @Test
   public void put_tableProfile_200(TestInfo test) throws HttpResponseException {
     Table table = createAndCheckTable(create(test), adminAuthHeaders());
-    List<String> columns = Arrays.asList("c1", "c2", "c3");
     ColumnProfile c1Profile = new ColumnProfile().withName("c1").withMax("100.0")
             .withMin("10.0").withUniqueCount(100.0);
     ColumnProfile c2Profile = new ColumnProfile().withName("c2").withMax("99.0").withMin("20.0").withUniqueCount(89.0);
@@ -765,7 +898,7 @@ public class TableResourceTest extends CatalogApplicationTest {
 
   /**
    * For cursor based pagination and implementation details:
-   * @see org.openmetadata.catalog.util.ResultList#ResultList(List, int, String, String)
+   * @see org.openmetadata.catalog.util.ResultList#ResultList
    *
    * The tests and various CASES referenced are base on that.
    */
@@ -886,79 +1019,20 @@ public class TableResourceTest extends CatalogApplicationTest {
   @Test
   public void patch_tableColumnTags_200_ok(TestInfo test) throws HttpResponseException, JsonProcessingException {
     // Create table without description, table tags, tier, owner, tableType, and tableConstraints
-    Table table = createTable(create(test).withTableConstraints(null), adminAuthHeaders());
+    List<Column> columns = new ArrayList<>();
-    assertNull(table.getDescription());
+    columns.add(getColumn("c1", INT, USER_ADDRESS_TAG_LABEL));
-    assertNull(table.getOwner());
+    columns.add(getColumn("c2", BIGINT, USER_ADDRESS_TAG_LABEL));
-    assertNull(table.getTableType());
+    columns.add(getColumn("c3", FLOAT, USER_BANK_ACCOUNT_TAG_LABEL));
-    assertNull(table.getTableConstraints());
 
+    Table table = createTable(create(test).withColumns(columns), adminAuthHeaders());
 
-    Map<String, List<TagLabel>> columnTagMap = new HashMap<>();
+    // Update the columns
-    columnTagMap.put("c1", singletonList(USER_ADDRESS_TAG_LABEL));
+    columns.get(0).setTags(List.of(USER_ADDRESS_TAG_LABEL, USER_BANK_ACCOUNT_TAG_LABEL)); // Add a tag
-    columnTagMap.put("c2", singletonList(USER_ADDRESS_TAG_LABEL));
+    columns.get(1).setTags(List.of(USER_ADDRESS_TAG_LABEL));                              // No change in tag
-    columnTagMap.put("c3", singletonList(USER_BANK_ACCOUNT_TAG_LABEL));
+    columns.get(2).setTags(new ArrayList<>());                                            // Remove tag
 
-    validateColumnTags(table, columnTagMap);
+    table = patchTableColumnAttributesAndCheck(table, columns, adminAuthHeaders());
-
+    validateColumns(columns, table.getColumns());
-    List<Column> updatedColumns = Arrays.asList(
-            new Column().withName("c1").withColumnDataType(ColumnDataType.BIGINT)
-                    .withTags(List.of(USER_ADDRESS_TAG_LABEL, USER_BANK_ACCOUNT_TAG_LABEL)),
-            new Column().withName("c2").withColumnDataType(ColumnDataType.BIGINT)
-                    .withTags(singletonList(USER_ADDRESS_TAG_LABEL)),
-            new Column().withName("c3").withColumnDataType(ColumnDataType.BIGINT)
-                    .withTags(new ArrayList<>()));
-
-    table = patchTableColumnAttributesAndCheck(table, updatedColumns, adminAuthHeaders());
-    table.setOwner(USER_OWNER1); // Get rid of href and name returned in the response for owner
-    columnTagMap.put("c1", List.of(USER_ADDRESS_TAG_LABEL, USER_BANK_ACCOUNT_TAG_LABEL));
-    columnTagMap.put("c2", singletonList(USER_ADDRESS_TAG_LABEL));
-    columnTagMap.put("c3", new ArrayList<>());
-    validateColumnTags(table, columnTagMap);
-  }
-
-  @Test
-  public void patch_tableIDChange_400(TestInfo test) throws HttpResponseException, JsonProcessingException {
-    // Ensure table ID can't be changed using patch
-    Table table = createTable(create(test), adminAuthHeaders());
-    UUID oldTableId = table.getId();
-    String tableJson = JsonUtils.pojoToJson(table);
-    table.setId(UUID.randomUUID());
-    HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
-            patchTable(oldTableId, tableJson, table, adminAuthHeaders()));
-    assertResponse(exception, BAD_REQUEST, CatalogExceptionMessage.readOnlyAttribute(Entity.TABLE, "id"));
-  }
-
-  @Test
-  public void patch_tableNameChange_400(TestInfo test) throws HttpResponseException, JsonProcessingException {
-    // Ensure table name can't be changed using patch
-    Table table = createTable(create(test), adminAuthHeaders());
-    String tableJson = JsonUtils.pojoToJson(table);
-    table.setName("newName");
-    HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
-            patchTable(tableJson, table, adminAuthHeaders()));
-    assertResponse(exception, BAD_REQUEST, CatalogExceptionMessage.readOnlyAttribute(Entity.TABLE, "name"));
-  }
-
-  @Test
-  public void patch_tableRemoveDatabase_400(TestInfo test) throws HttpResponseException, JsonProcessingException {
-    // Ensure table database it belongs to can't be removed
-    Table table = createTable(create(test).withDatabase(DATABASE.getId()), adminAuthHeaders());
-    String tableJson = JsonUtils.pojoToJson(table);
-    table.setDatabase(null);
-    HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
-            patchTable(tableJson, table, adminAuthHeaders()));
-    assertResponse(exception, BAD_REQUEST, "Table relationship database can't be removed");
-  }
-
-  @Test
-  public void patch_tableReplaceDatabase_400(TestInfo test) throws HttpResponseException, JsonProcessingException {
-    // Ensure table database it belongs to can't be removed
-    Table table = createTable(create(test).withDatabase(DATABASE.getId()), adminAuthHeaders());
-    String tableJson = JsonUtils.pojoToJson(table);
-    table.getDatabase().setId(UUID.randomUUID());
-    HttpResponseException exception = assertThrows(HttpResponseException.class, () ->
-            patchTable(tableJson, table, adminAuthHeaders()));
-    assertResponse(exception, BAD_REQUEST, "Table relationship database can't be replaced");
   }
 
   @Test
@@ -1011,12 +1085,12 @@ public class TableResourceTest extends CatalogApplicationTest {
 
     // Validate information returned in patch response has the updates
     Table updatedTable = patchTable(tableJson, table, authHeaders);
-    validateTable(updatedTable, table.getDescription(), owner, null, tableType,
+    validateTable(updatedTable, table.getDescription(), table.getColumns(), owner, null, tableType,
             tableConstraints, tags);
 
     // GET the table and Validate information returned
     Table getTable = getTable(table.getId(), "owner,tableConstraints,columns, tags", authHeaders);
-    validateTable(getTable, table.getDescription(), owner, null, tableType, tableConstraints, tags);
+    validateTable(getTable, table.getDescription(), table.getColumns(), owner, null, tableType, tableConstraints, tags);
     return updatedTable;
   }
 
@@ -1028,8 +1102,7 @@ public class TableResourceTest extends CatalogApplicationTest {
     table.setColumns(columns);
 
     // Validate information returned in patch response has the updates
-    Table updatedTable = patchTable(tableJson, table, authHeaders);
+    return patchTable(tableJson, table, authHeaders);
-    return updatedTable;
   }
 
   // TODO disallow changing href, usage
@@ -1041,13 +1114,12 @@ public class TableResourceTest extends CatalogApplicationTest {
           throws HttpResponseException {
     // Validate table created has all the information set in create request
     Table table = createTable(create, authHeaders);
-    validateTable(table, create.getDescription(), create.getOwner(),
+    validateTable(table, create.getDescription(), create.getColumns(), create.getOwner(),
             create.getDatabase(), create.getTableType(), create.getTableConstraints(), create.getTags());
-    validateTags(create.getTags(), table.getTags());
 
     // GET table created and ensure it has all the information set in create request
-    Table getTable = getTable(table.getId(), "owner,database,tags,tableConstraints", authHeaders);
+    Table getTable = getTable(table.getId(), "columns,owner,database,tags,tableConstraints", authHeaders);
-    validateTable(getTable, create.getDescription(), create.getOwner(),
+    validateTable(getTable, create.getDescription(), create.getColumns(), create.getOwner(),
             create.getDatabase(), create.getTableType(), create.getTableConstraints(), create.getTags());
 
     // If owner information is set, GET and make sure the user or team has the table in owns list
@@ -1129,24 +1201,28 @@ public class TableResourceTest extends CatalogApplicationTest {
     }
     // When tags from the expected list is added to an entity, the derived tags for those tags are automatically added
     // So add to the expectedList, the derived tags before validating the tags
-    List<TagLabel> updatedExpectedList = new ArrayList<>();
+    List<TagLabel> updatedExpectedList = new ArrayList<>(expectedList);
-    updatedExpectedList.addAll(expectedList);
     for (TagLabel expected : expectedList) {
       List<TagLabel> derived = EntityUtil.getDerivedTags(expected, TagResourceTest.getTag(expected.getTagFQN(),
               adminAuthHeaders()));
       updatedExpectedList.addAll(derived);
     }
     updatedExpectedList = updatedExpectedList.stream().distinct().collect(Collectors.toList());
+    updatedExpectedList.sort(new TagLabelComparator());
+    actualList.sort(new TagLabelComparator());
 
-    assertTrue(actualList.containsAll(updatedExpectedList));
+    assertEquals(updatedExpectedList.size(), actualList.size());
-    assertTrue(updatedExpectedList.containsAll(actualList));
+    for (int i = 0; i < actualList.size(); i++) {
+      assertEquals(updatedExpectedList.get(i), actualList.get(i));
+    }
   }
 
   public static Table createTable(CreateTable create, Map<String, String> authHeaders) throws HttpResponseException {
     return TestUtils.post(CatalogApplicationTest.getResource("tables"), create, Table.class, authHeaders);
   }
 
-  private static void validateTable(Table table, String expectedDescription, EntityReference expectedOwner,
+  private static void validateTable(Table table, String expectedDescription,
+                                    List<Column> expectedColumns, EntityReference expectedOwner,
                                     UUID expectedDatabaseId, TableType expectedTableType,
                                     List<TableConstraint> expectedTableConstraints, List<TagLabel> expectedTags)
           throws HttpResponseException {
@@ -1156,6 +1232,8 @@ public class TableResourceTest extends CatalogApplicationTest {
     assertEquals(expectedDescription, table.getDescription());
     assertEquals(expectedTableType, table.getTableType());
 
+    validateColumns(expectedColumns, table.getColumns());
+
     // Validate owner
     if (expectedOwner != null) {
       TestUtils.validateEntityReference(table.getOwner());
@@ -1170,17 +1248,37 @@ public class TableResourceTest extends CatalogApplicationTest {
       assertEquals(expectedDatabaseId, table.getDatabase().getId());
     }
 
+
     // Validate table constraints
     assertEquals(expectedTableConstraints, table.getTableConstraints());
     validateTags(expectedTags, table.getTags());
     TestUtils.validateEntityReference(table.getFollowers());
   }
 
-  private static void validateColumnTags(Table table, Map<String, List<TagLabel>> columnTagMap)
+  private static void validateColumn(Column expectedColumn, Column actualColumn) throws HttpResponseException {
-          throws HttpResponseException {
+    assertNotNull(actualColumn.getFullyQualifiedName());
-    for (Column column: table.getColumns()) {
+    assertEquals(expectedColumn.getName(), actualColumn.getName());
-      List<TagLabel> expectedTags = columnTagMap.get(column.getName());
+    assertEquals(expectedColumn.getDataType(), actualColumn.getDataType());
-      validateTags(expectedTags, column.getTags());
+    assertEquals(expectedColumn.getArrayDataType(), actualColumn.getArrayDataType());
+    if (expectedColumn.getDataTypeDisplay() != null) {
+      assertEquals(expectedColumn.getDataTypeDisplay().toLowerCase(Locale.ROOT), actualColumn.getDataTypeDisplay());
+    }
+    validateTags(expectedColumn.getTags(), actualColumn.getTags());
+
+    // Check the nested columns
+    validateColumns(expectedColumn.getChildren(), actualColumn.getChildren());
+  }
+
+  private static void validateColumns(List<Column> expectedColumns, List<Column> actualColumns) throws HttpResponseException {
+    if (expectedColumns == null && actualColumns == null) {
+      return;
+    }
+    // Sort columns by name
+    expectedColumns.sort(new ColumnComparator());
+    actualColumns.sort(new ColumnComparator());
+    assertEquals(expectedColumns.size(), actualColumns.size());
+    for (int i = 0; i < expectedColumns.size(); i++) {
+      validateColumn(expectedColumns.get(i), actualColumns.get(i));
     }
   }
 
@@ -1249,13 +1347,13 @@ public class TableResourceTest extends CatalogApplicationTest {
   public static Table updateAndCheckTable(CreateTable create, Status status, Map<String, String> authHeaders)
           throws HttpResponseException {
     Table updatedTable = updateTable(create, status, authHeaders);
-    validateTable(updatedTable, create.getDescription(), create.getOwner(), create.getDatabase(),
+    validateTable(updatedTable, create.getDescription(), create.getColumns(), create.getOwner(), create.getDatabase(),
             create.getTableType(), create.getTableConstraints(), create.getTags());
 
     // GET the newly updated database and validate
-    Table getTable = getTable(updatedTable.getId(), "database,owner,tableConstraints,tags", authHeaders);
+    Table getTable = getTable(updatedTable.getId(), "columns,database,owner,tableConstraints,tags", authHeaders);
-    validateTable(getTable, create.getDescription(), create.getOwner(), create.getDatabase(), create.getTableType(),
+    validateTable(getTable, create.getDescription(), create.getColumns(), create.getOwner(), create.getDatabase(),
-            create.getTableConstraints(), create.getTags());
+            create.getTableType(), create.getTableConstraints(), create.getTags());
     // TODO columns check
     return updatedTable;
   }
@@ -1297,7 +1395,6 @@ public class TableResourceTest extends CatalogApplicationTest {
                            Map<String, String> authHeaders) throws JsonProcessingException, HttpResponseException {
     String updateTableJson = JsonUtils.pojoToJson(updatedTable);
     JsonPatch patch = JsonSchemaUtil.getJsonPatch(originalJson, updateTableJson);
-    LOG.info("Applying patch ", patch);
     return TestUtils.patch(CatalogApplicationTest.getResource("tables/" + tableId),
             patch, Table.class, authHeaders);
   }

catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/feeds/FeedResourceTest.java

@@ -69,7 +69,7 @@ public class FeedResourceTest extends CatalogApplicationTest {
     TableResourceTest.setup(test); // Initialize TableResourceTest for using helper methods
     CreateTable createTable = TableResourceTest.create(test);
     TABLE = createAndCheckTable(createTable, adminAuthHeaders());
-    COLUMNS = Collections.singletonList(new Column().withName("column1").withColumnDataType(ColumnDataType.BIGINT));
+    COLUMNS = Collections.singletonList(new Column().withName("column1").withDataType(ColumnDataType.BIGINT));
     TABLE_LINK = String.format("<#E/table/%s>", TABLE.getFullyQualifiedName());
 
     USER = TableResourceTest.USER1;