anuragmantri commented on code in PR #18534:
URL: https://github.com/apache/iceberg/pull/18534#discussion_r4321711328
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HMSTablePropertyHelper.java:
##########
@@ -110,6 +110,7 @@ public static void updateHmsTableForIcebergTable(
maxHiveTablePropertySize);
setStorageHandler(parameters, hiveEngineEnabled);
setMetadataHash(metadata, parameters);
+ parameters.put(TableProperties.FORMAT_VERSION,
String.valueOf(metadata.formatVersion()));
Review Comment:
`setCommonParameters` is also used for views, and a view has its own format
version in `ViewMetadata`. Writing it there would put a view version under the
same key as a table version. I kept it in `updateHmsTableForIcebergTable` so
only tables get it. If you would rather have it in the shared method, I can
pass the format version in and use a different key for views. What do you
prefer?
##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -994,6 +994,54 @@ public void testUUIDinTableProperties() throws Exception {
}
}
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
Review Comment:
Done. The test now uses `TestHelpers#ALL_VERSIONS`, so it also covers v4.
That needed the table location as a `file:` URI, because v4 requires an
absolute location.
--
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]
