henrib commented on code in PR #6936:
URL: https://github.com/apache/hive/pull/6936#discussion_r4490491263
##########
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:
Yes, backward compatibility: `HiveCatalog` (here and upstream Iceberg) has
always taken the owner from `hive.metastore.table.owner` when set, and engines
embedding it (Spark, Flink) rely on that. The HMS REST catalog doesn't use this
default; it sets the property from
`metastore.iceberg.catalog.client-supplied-owner.enabled`, which is off by
default.
##########
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:
The two differ in intent: `resolveTableOwner` decides the owner to *record*
at creation and applies the client-supplied-owner setting. Here, the property
is the owner the HMS REST catalog *reports* (the one HMS recorded), so the
setting shouldn't apply. If it's off, `resolveTableOwner` would replace the
reported owner with the local user. I can make the comment clearer about that.
##########
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:
It's how the REST client learns the `Authorized` state HMS recorded, which
it can't read directly. Without it, the client would have to treat every
REST-loaded view as `Authorized=false`, so readers would need privileges on the
underlying tables even for views created through HiveServer2
(`viewCreatedByHiveServer2StaysAuthorized`), or rely on view metadata
properties, which clients can set.
--
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]
