singhpk234 commented on code in PR #4546:
URL: https://github.com/apache/iceberg/pull/4546#discussion_r849465310
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -559,4 +556,32 @@ public void testSetSnapshotSummary() throws Exception {
spyOps.setSnapshotSummary(parameters, snapshot);
Assert.assertEquals("The snapshot summary must not be in parameters due to
the size limit", 0, parameters.size());
}
+
+ @Test
+ public void testSetDefaultPartitionSpec() throws Exception {
+ Schema schema = new Schema(
+ required(1, "id", Types.IntegerType.get(), "unique ID"),
+ required(2, "data", Types.StringType.get())
+ );
+ PartitionSpec spec = PartitionSpec.builderFor(schema)
+ .bucket("data", 16)
+ .build();
+ TableIdentifier tableIdent = TableIdentifier.of(DB_NAME, "tbl");
+
+ try {
+ catalog.buildTable(tableIdent, schema)
+ .withPartitionSpec(spec)
+ .create();
+
+ Assert.assertEquals(PartitionSpecParser.toJson(spec),
Review Comment:
[minor] how about asserting the case where we don't set default partition
spec un-partitioned spec
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -433,6 +437,20 @@ void setSnapshotSummary(Map<String, String> parameters,
Snapshot currentSnapshot
}
}
+ private void setPartitionSpec(TableMetadata metadata, Map<String, String>
parameters) {
+ parameters.remove(TableProperties.DEFAULT_PARTITION_SPEC);
+ if (metadata.spec() != null && metadata.spec().isPartitioned()) {
Review Comment:
[question] In v1 spec when a partition is dropped it is replaced by
VoidTransform, if all the transforms are void we should consider it
un-partitioned (This may be beyond the scope of present PR), but presently when
we call isPartitioned it will return true in this case. do we want to store
partition spec in this scenario ? Your thoughts.
This is based on ticket https://github.com/apache/iceberg/issues/3014
@RussellSpitzer filed a while back.
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -244,13 +247,16 @@ public void testCreateTableDefaultSortOrder() {
Table table = catalog.createTable(tableIdent, schema, spec);
Assert.assertEquals("Order ID must match", 0,
table.sortOrder().orderId());
Assert.assertTrue("Order must unsorted", table.sortOrder().isUnsorted());
+
+ Assert.assertTrue("Must not have default sort order in catalog",
+ !hmsTableParameters().containsKey(DEFAULT_SORT_ORDER));
Review Comment:
[nit] can use assertFalse instead.
--
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]
