okumin commented on code in PR #6592:
URL: https://github.com/apache/hive/pull/6592#discussion_r3576480421
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/properties/HMSPropertyManager.java:
##########
@@ -25,14 +25,20 @@
import java.util.TreeMap;
/**
- * A property manager tailored for the HiveMetaStore.
+ * A property manager tailored for the HiveMetaStore operating on Iceberg
tables.
* It describes properties for cluster, database and table based on declared
schemas.
* A property is of the form:
* <ul>
* <li>name : when it refers to a cluster property named 'name'</li>
* <li>db.name : when it refers to a database property named 'name' for the
database 'db'</li>
* <li>db.table.name : when it refers to a table property named 'name' for
the table 'table' in the database 'db'</li>
* </ul>
+ * <p>
+ * This is meant as the base of a property manager that could be used to
manage properties in the HiveMetaStore
+ * for Iceberg table maintenance operations.
+ * It is not meant to be a complete implementation, but rather a starting
point for further development.
+ * A potential extension would be to add maintenance operation properties at
the cluster and database levels to the schema
+ * so they could act as default values for table properties used by
maintenance operations.
Review Comment:
```suggestion
* A potential extension would be to add maintenance operation properties
at the cluster and database levels to the
* schema so they could act as default values for table properties used by
maintenance operations.
```
Just for the 120-characters rule
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/properties/HMSPropertyManager.java:
##########
@@ -25,14 +25,20 @@
import java.util.TreeMap;
/**
- * A property manager tailored for the HiveMetaStore.
+ * A property manager tailored for the HiveMetaStore operating on Iceberg
tables.
* It describes properties for cluster, database and table based on declared
schemas.
* A property is of the form:
* <ul>
* <li>name : when it refers to a cluster property named 'name'</li>
* <li>db.name : when it refers to a database property named 'name' for the
database 'db'</li>
* <li>db.table.name : when it refers to a table property named 'name' for
the table 'table' in the database 'db'</li>
* </ul>
+ * <p>
+ * This is meant as the base of a property manager that could be used to
manage properties in the HiveMetaStore
Review Comment:
I am just curious about the future vision stated here
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/properties/HMSPropertyManager.java:
##########
@@ -45,14 +51,13 @@ public class HMSPropertyManager extends PropertyManager {
/** Table maintenance operation type. */
public enum MaintenanceOpType {
- COMPACTION,
- SNAPSHOT_EXPIRY,
- STATS_REBUILD,
- MV_BUILD,
- MV_REFRESH,
- SHUFFLE_TO_NEW_PART,
- RECOMPRESS,
- REORG
+ EXPIRE_SNAPSHOT,
+ REWRITE_DATA_FILES,
+ COMPRESS_DATA_FILES,
+ REWRITE_MANIFEST,
+ DELETE_ORPHAN_FILES,
+ REWRITE_POSITION_DELETE,
+ COMPUTE_TABLE_STATS
Review Comment:
Could you explain the enum changes? I understand we can change them as the
original values are not used yet. However, I am not confident about why the new
values are better than the old ones. One option is to separate the PR and
involve Dmitriy or someone more familiar with maintenance tasks there.
--
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]
