illiabarbashov-sketch commented on code in PR #6936:
URL: https://github.com/apache/hive/pull/6936#discussion_r4457759939


##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java:
##########
@@ -213,12 +213,22 @@ public static void applyIcebergViewToHmsTable(Table 
hmsTable, View view, Configu
         maxHiveTablePropertySize,
         null,
         sqlText);
+    // Take the Authorized state reported with this view version, the one the 
SQL above comes from. A reported
+    // state only ever marks the view; it never clears an Authorized=false 
parameter already set.
+    String authorized = 
view.properties().get(HiveCatalog.VIEW_AUTHORIZED_PROPERTY);

Review Comment:
   [nitpick] I don't know if that's better but it could be separated into a 
single method with boolean flag and could be used also in 
`HiveRESTCatalogClient` 
   like this: 
   ```
   public static void applyReportedViewAuthorization(View view, Table hmsView, 
boolean, markWhenUnreported) {
       String authorized = 
view.properties().get(HiveCatalog.VIEW_AUTHORIZED_PROPERTY);
       if ((authorized != null || markWhenUnreported) && 
!"true".equalsIgnoreCase(authorized)) {
         hmsView.getParameters().put(HiveCatalog.HMS_VIEW_AUTHORIZED, "false");
       }
     }
   ```
   
   One advantage is that you can add javadoc in one place instead of two 
separate comments sections.  



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

Reply via email to