[GitHub] [iceberg] jackye1995 commented on a diff in pull request #7300: Spark: Show Create Round trip tests

["jackye1995 (via GitHub)" <gi...@apache.org>]

jackye1995 commented on code in PR #7300:
URL: https://github.com/apache/iceberg/pull/7300#discussion_r1161851380


##########
spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java:
##########
@@ -341,4 +414,65 @@ public void testDowngradeTableToFormatV1ThroughTablePropertyFails() {
         "Cannot downgrade v2 table to v1",
         () -> sql("ALTER TABLE %s SET TBLPROPERTIES ('format-version'='1')", tableName));
   }
+
+  private void validateExpectedShowCreateTable(
+      String tableName,
+      String expectedSchema,
+      String expectedLocation,
+      Map<String, String> expectedProperties) {
+    validateExpectedShowCreateTable(
+        tableName, expectedSchema, expectedLocation, expectedProperties, null, null, null);
+  }
+
+  private void validateExpectedShowCreateTable(
+      String tableName,
+      String expectedSchema,
+      String expectedLocation,
+      Map<String, String> expectedProperties,
+      String comment) {
+    validateExpectedShowCreateTable(
+        tableName, expectedSchema, expectedLocation, expectedProperties, null, null, comment);
+  }
+
+  private void validateExpectedShowCreateTable(
+      String tableName,
+      String expectedSchema,
+      String expectedLocation,
+      Map<String, String> expectedProperties,
+      String partitionClause,
+      String bucketClause,
+      String comment) {
+    StringBuilder expectedCreate = new StringBuilder();
+
+    expectedCreate.append(
+        String.format(
+            "CREATE TABLE %s ",
+            (catalogName.equals("spark_catalog") ? "spark_catalog." : "") + tableName));
+
+    expectedCreate.append(expectedSchema + "\n");
+    expectedCreate.append("USING iceberg\n");
+    if (comment != null) {
+      expectedCreate.append(comment + "\n");
+    }
+
+    if (partitionClause != null) {
+      expectedCreate.append(partitionClause + "\n");
+    }
+
+    if (bucketClause != null) {
+      expectedCreate.append(bucketClause + "\n");
+    }
+
+    expectedCreate.append(String.format("LOCATION '%s'\n", expectedLocation));
+    expectedCreate.append("TBLPROPERTIES (\n  ");
+    expectedCreate.append(tablePropsAsString(expectedProperties, " = ", ",\n  "));
+    expectedCreate.append(")\n");
+
+    List<Object[]> actualCreate = sql("SHOW CREATE table %s", tableName);
+
+    assertEquals(
+        "Should have expected create",
+        ImmutableList.of(row(expectedCreate.toString())),
+        actualCreate);

Review Comment:
   +1, string check is too fragile. 
   
   It might be tricky to do full comparison of the table though, we have methods like `Schema.sameSchema` to check schema equality, maybe we should fill in the missing equality check methods for all relevant components of the table, so we can do a `table.sameTable(anotherTable)`



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org