[GitHub] [iceberg] zhangdove commented on a change in pull request #1850: Hive: fix hiveCatalog create/write table and hive client read no data

[GitBox <gi...@apache.org>]

zhangdove commented on a change in pull request #1850:
URL: https://github.com/apache/iceberg/pull/1850#discussion_r539183753



##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
##########
@@ -366,20 +370,27 @@ static void validateTableIsIceberg(Table table, String fullName) {
    * The decision is made like this:
    * <ol>
    * <li>Table property value {@link TableProperties#ENGINE_HIVE_ENABLED}
-   * <li>If the table property is not set then check the hive-site.xml property value
+   * <li>If the table property is not set then check hive metastore TABLE_PARAMS value
    * {@link ConfigProperties#ENGINE_HIVE_ENABLED}
+   * <li>If the table property is not set and hive metastore TABLE_PARAMS is not set
+   * then check the hive-site.xml property value{@link ConfigProperties#ENGINE_HIVE_ENABLED}
    * <li>If none of the above is enabled then use the default value {@link TableProperties#ENGINE_HIVE_ENABLED_DEFAULT}
    * </ol>
    * @param metadata Table metadata to use
+   * @param parameters Hive table parameters to use
    * @param conf The hive configuration to use
    * @return if the hive engine related values should be enabled or not
    */
-  private static boolean hiveEngineEnabled(TableMetadata metadata, Configuration conf) {
+  private static boolean hiveEngineEnabled(TableMetadata metadata, Map<String, String> parameters, Configuration conf) {

Review comment:
       @pvary I've seen some discussion(it’s great work), but based on that, could we do the following things here?
   
   1. modify Hive related [docs](https://github.com/apache/iceberg/blob/master/site/docs/hive.md#hive-configuration), at least in terms of usage? (disable ENGINE_HIVE_ENABLED, and prompt the user to use [another way](https://github.com/apache/iceberg/blob/master/site/docs/hive.md#table-property-configuration) )
   
   2. save the value of `ENGINE_HIVE_ENABLED` in the Configuration into the "Iceberg table property"
   
   Of course,if there is a better solution, I am very interested in how to deal with hive engine enable priorities, and close this pr.
   
   Thanks.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org