okumin commented on code in PR #6936:
URL: https://github.com/apache/hive/pull/6936#discussion_r4489687556
##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveHadoopUtil.java:
##########
@@ -28,6 +30,15 @@ public class HiveHadoopUtil {
private static final Logger LOG =
LoggerFactory.getLogger(HiveHadoopUtil.class);
+ /**
+ * Whether the HMS owner of a table or view created through this catalog may
be taken from the client-supplied
+ * property {@link HiveCatalog#HMS_TABLE_OWNER} instead of the authenticated
caller. On by default, as engines
+ * embedding the catalog (Spark, Flink, ...) rely on it. The HMS REST
catalog always sets it, from
+ * {@code metastore.iceberg.catalog.client-supplied-owner.enabled} (off by
default).
+ */
+ public static final String CLIENT_SUPPLIED_OWNER_ENABLED =
"iceberg.hive.client-supplied-owner.enabled";
+ public static final boolean CLIENT_SUPPLIED_OWNER_ENABLED_DEFAULT = true;
Review Comment:
Why is it true by default? Backward compatibility?
##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java:
##########
@@ -213,19 +213,42 @@ 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
+ applyReportedViewAuthorization(view, hmsTable, false);
hmsTable.setCreateTime((int) (metadata.version(1).timestampMillis() /
1000));
hmsTable.setLastAccessTime((int)
(metadata.currentVersion().timestampMillis() / 1000));
- hmsTable.setOwner(
- PropertyUtil.propertyAsString(
- metadata.properties(), HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()));
+ // Keep the owner HMS recorded. Only a view built without HMS (REST
catalog client) takes it from the property,
+ // which the HMS REST catalog sets to the recorded owner.
+ if (StringUtils.isEmpty(hmsTable.getOwner())) {
+ hmsTable.setOwner(
+ PropertyUtil.propertyAsString(
+ metadata.properties(), HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()));
Review Comment:
Can't we use `HiveHadoopUtil.resolveTableOwner` one here?
##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -93,6 +93,47 @@ public class HiveCatalog extends BaseMetastoreViewCatalog
public static final String EXTERNAL_WAREHOUSE_LOCATION =
"external-warehouse";
public static final String HMS_TABLE_OWNER = "hive.metastore.table.owner";
+
+ /**
+ * HMS view parameter that HiveMetaStoreAuthorizer sets to {@code false} on
views created or altered by a
+ * non-superuser, making them deferred-authorization views: HiveServer2
checks the reader's privileges on the
+ * tables such a view references.
+ * <p>The marker is sticky through this catalog: an update keeps the
parameters HMS recorded, and a superuser's
+ * update does not clear the marker. Once a non-superuser has written a
view's SQL, readers keep being
+ * authorized on the underlying tables; only dropping and re-creating the
view as a superuser lifts it.</p>
+ */
+ public static final String HMS_VIEW_AUTHORIZED = "Authorized";
+
+ /**
+ * View property through which the HMS REST catalog reports {@link
#HMS_VIEW_AUTHORIZED} to its clients,
+ * as {@code true} or {@code false}. The server drops it from requests and
sets it in its responses from the
+ * value recorded in HMS; neither this property nor {@link
#HMS_VIEW_AUTHORIZED} is copied from view properties
+ * into HMS.
+ * <p>Clients use it only to mark a view {@link #HMS_VIEW_AUTHORIZED}{@code
=false}: any value other than
+ * {@code true} marks the view, and {@code true} never clears a marker
already set.</p>
+ */
+ public static final String VIEW_AUTHORIZED_PROPERTY =
"hive.metastore.view.authorized";
Review Comment:
I'm checking what if we don't have this new marker property; it might be
helpful if we have the entire authz mechanism
--
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]
