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


##########
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:
   Cool will update. IT looks like table equality isn't formally defined yet, 
but at least for purpose of this test we can add our own validation for the 
relevant table fields. 
   Also one thing to note is that Spark Show create tests 
https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/ShowCreateTableSuite.scala
 perform a string check, but I don't think we need to follow that here because 
for Iceberg we have more combinations that will make the string fragile



##########
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:
   Cool will update. It looks like table equality isn't formally defined yet, 
but at least for purpose of this test we can add our own validation for the 
relevant table fields. 
   Also one thing to note is that Spark Show create tests 
https://github.com/apache/spark/blob/master/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/ShowCreateTableSuite.scala
 perform a string check, but I don't think we need to follow that here because 
for Iceberg we have more combinations that will make the string fragile



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to