wangyum commented on code in PR #6888:
URL: https://github.com/apache/hive/pull/6888#discussion_r4310533334
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1577,6 +1577,10 @@ public GetPartitionsResponse
get_partitions_with_specs(GetPartitionsRequest requ
getTableRequest.setCatName(catName);
Table table = get_table_core(getTableRequest);
firePreEvent(new PreReadTableEvent(table, this));
+ // Perform the same table-level filter hook check the regular partition
listing RPCs
+ // perform in GetPartitionsHandler.authorizeTableForPartitionMetadata()
+ FilterUtils.checkDbAndTableFilters(isServerFilterEnabled, filterHook,
normalizeIdentifier(catName),
Review Comment:
Thanks for looking at this! I don't think it's redundant —
`PreReadTableEvent` and `FilterUtils.checkDbAndTableFilters` are two separate
mechanisms:
- `firePreEvent(new PreReadTableEvent(...))` triggers pre-event listeners
like `AuthorizationPreEventListener`, which does privilege-based authorization
(throws `AuthorizationException` if the caller lacks `SELECT`).
- `FilterUtils.checkDbAndTableFilters` calls the configured
`MetaStoreFilterHook` (e.g. Ranger) for *visibility* filtering — it hides
objects the caller shouldn't even know exist, by throwing
`NoSuchObjectException`.
These aren't substitutes for each other, and the existing
`GetPartitionsHandler.beforeExecute()` already calls both back-to-back
(`firePreEvent` then `authorizeTableForPartitionMetadata()`), so this PR is
just matching that established pattern rather than introducing a new one.
**Concrete example**: say Ranger is configured as the filter hook
(`metastore.server.filter.enabled=true`), and user `bob` has no `SELECT` grant
on `sales.transactions`.
- Calling `get_partitions` today: `authorizeTableForPartitionMetadata()`
filters the table name via the Ranger hook, gets an empty result, and fails
fast with `NoSuchObjectException` — before touching any partition data.
- Calling `get_partitions_with_specs` *before this fix*: HMS fetches the
`Table` object, fires `PreReadTableEvent` (a no-op if no legacy
`AuthorizationPreEventListener` is configured, which is common in Ranger-only
setups), then goes ahead and fetches **all** matching partitions from the
RawStore, and only filters them out afterward at the partition level. `bob`
gets back an empty list instead of `NoSuchObjectException` — inconsistent with
every other partition-listing RPC, and wasteful if the table has a large number
of partitions.
This PR closes that gap by performing the same table-level check
`get_partitions` already does, right after the `PreReadTableEvent`, so
authorization ordering is preserved (pre-event listeners still run first) but
the filter hook now also gets a chance to fail fast.
The test added in `TestFilterHooks.testFilterForPartition` covers exactly
this: with a blocking filter hook, it asserts `getPartitionsWithSpecs()` throws
`NoSuchObjectException` at the server side — which would fail if this check
were removed.
--
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]
