ebyhr commented on code in PR #15993:
URL: https://github.com/apache/iceberg/pull/15993#discussion_r3090009591
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/RegisterTableProcedure.java:
##########
@@ -46,9 +46,11 @@ class RegisterTableProcedure extends BaseProcedure {
requiredInParameter("table", DataTypes.StringType);
private static final ProcedureParameter METADATA_FILE_PARAM =
requiredInParameter("metadata_file", DataTypes.StringType);
+ private static final ProcedureParameter OVERWRITE_PARAM =
+ optionalInParameter("overwrite", DataTypes.BooleanType, "false");
Review Comment:
Could you update `docs/docs/spark-procedures.md`?
##########
spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java:
##########
@@ -82,4 +83,59 @@ public void testRegisterTable() throws NoSuchTableException,
ParseException {
.as("Should have the right datafile count in the procedure result")
.contains(originalFileCount, atIndex(2));
}
+
+ @TestTemplate
+ public void testRegisterTableAlreadyExistsFails() throws
NoSuchTableException, ParseException {
+ long numRows = 1000;
+
+ sql("CREATE TABLE %s (id int, data string) using ICEBERG", tableName);
+ spark
+ .range(0, numRows)
+ .withColumn("data", functions.col("id").cast(DataTypes.StringType))
+ .writeTo(tableName)
+ .append();
Review Comment:
We could remove this append. An empty table is sufficient to verify "Table
already exists" failure.
##########
spark/v4.1/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java:
##########
@@ -82,4 +83,59 @@ public void testRegisterTable() throws NoSuchTableException,
ParseException {
.as("Should have the right datafile count in the procedure result")
.contains(originalFileCount, atIndex(2));
}
+
+ @TestTemplate
+ public void testRegisterTableAlreadyExistsFails() throws
NoSuchTableException, ParseException {
+ long numRows = 1000;
+
+ sql("CREATE TABLE %s (id int, data string) using ICEBERG", tableName);
+ spark
+ .range(0, numRows)
+ .withColumn("data", functions.col("id").cast(DataTypes.StringType))
+ .writeTo(tableName)
+ .append();
+
+ Table table = Spark3Util.loadIcebergTable(spark, tableName);
+ String metadataJson = TableUtil.metadataFileLocation(table);
+
+ sql("CALL %s.system.register_table('%s', '%s')", catalogName, targetName,
metadataJson);
+
+ assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.register_table('%s', '%s')",
+ catalogName, targetName, metadataJson))
+ .isInstanceOf(Exception.class)
+ .hasMessageContaining("Table already exists");
+ }
+
+ @TestTemplate
+ public void testRegisterTableWithOverwriteNotSupported()
+ throws NoSuchTableException, ParseException {
Review Comment:
nit: We could change to `throws Exception`. It's just a test method, so
listing all exceptions isn't necessary.
--
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]
