henrib commented on code in PR #6936:
URL: https://github.com/apache/hive/pull/6936#discussion_r4458302422
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -513,8 +540,108 @@ private RESTResponse dropView(Map<String, String> vars) {
private LoadViewResponse registerView(Map<String, String> vars, Object body)
{
Namespace namespace = namespaceFromPathVars(vars);
+ // The registered metadata file is not rewritten, so it keeps any
HMS-managed properties; they are not copied
+ // into HMS (HMSTablePropertyHelper) and are set from HMS in the response.
RegisterViewRequest request = castRequest(RegisterViewRequest.class, body);
- return CatalogHandlers.registerView(asViewCatalog, namespace, request);
+ return withViewAuthorization(TableIdentifier.of(namespace, request.name()),
+ CatalogHandlers.registerView(asViewCatalog, namespace, request));
+ }
+
+ /**
+ * Whether a view property is reserved for state HMS manages: the
authorization marker, under either name, and
+ * the owner unless client-supplied owners are enabled.
+ */
+ private boolean isReservedViewProperty(String key) {
+ return HiveCatalog.isViewAuthorizationProperty(key) ||
+ (!clientSuppliedOwnerEnabled &&
HiveCatalog.HMS_TABLE_OWNER.equalsIgnoreCase(key));
+ }
+
+ private Map<String, String> withoutReservedProperties(Map<String, String>
properties) {
+ Map<String, String> kept = new HashMap<>(properties);
+ kept.keySet().removeIf(this::isReservedViewProperty);
+ return kept;
+ }
+
+ /**
+ * Drops the reserved properties (see {@link #isReservedViewProperty}) from
a view creation, so that they are not
+ * stored in the Iceberg view metadata file. Dropping rather than rejecting
keeps clients that send back the
+ * properties of a loaded view working.
+ */
+ private CreateViewRequest withoutReservedProperties(CreateViewRequest
request) {
+ if
(request.properties().keySet().stream().noneMatch(this::isReservedViewProperty))
{
+ return request;
+ }
+ return ImmutableCreateViewRequest.builder()
+ .from(request)
+ .properties(withoutReservedProperties(request.properties()))
+ .build();
+ }
+
+ /** Drops the reserved properties from the property updates of a view
update; see the creation overload. */
+ private UpdateTableRequest withoutReservedProperties(UpdateTableRequest
request) {
Review Comment:
Moved to a new `HMSViewProperties` class in cb6181544d: it holds the
reserved-property filtering and the view state reported in responses; the
adapter only delegates. I kept it separate from `IcebergAuthorizer` since it
doesn't make authorization decisions, it filters and reports HMS-managed
properties.
##########
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:
Done in cb6181544d: `MetastoreUtil.applyReportedViewAuthorization(view,
hmsView, markWhenUnreported)`, used by both paths with a single javadoc.
--
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]
