xiangfu0 commented on code in PR #20169:
URL: https://github.com/apache/pinot/pull/20169#discussion_r4487397939
##########
pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java:
##########
@@ -579,11 +589,48 @@ private void handleSubquery(ExpressionContext expression,
TableExecutionInfo exe
arguments.set(1,
ExpressionContext.forLiteral(RequestUtils.getLiteral(((IdSet)
result).toBase64String())));
} else {
for (ExpressionContext argument : arguments) {
- handleSubquery(argument, executionInfo, timerContext, executorService,
endTimeMs);
+ handleSubquery(argument, executionInfo, timerContext, executorService,
queryContext);
}
}
}
+ /// Adds the row-level security filter of the queried table to the subquery
of an `IN_PARTITIONED_SUBQUERY`.
+ ///
+ /// The broker adds the row filters of the principal to the WHERE clause of
the query it sends and stamps them in
+ /// the query options under the key of the table it authorized (see
[RlsUtils#buildRlsFilterKey(String)]), but the
+ /// subquery reaches the server as an opaque string literal that carries
neither, so without this it would build
+ /// its ID_SET from rows the principal is not allowed to see. The subquery
runs on the segments selected for the
+ /// outer query whatever table its FROM clause names, so the filter of the
outer table is the one that applies: the
+ /// data source is renamed to that table so that the rewriter finds the
filter even when the FROM clause names
+ /// another table. The filter is added to the options the subquery was
written with, overriding any row filter
+ /// smuggled into them, so that a subquery nested in it is filtered the same
way. A query that carries row filters
+ /// but none for the queried table, which an older broker produces when a
materialized view rewrite swaps the table
+ /// after the filters were stamped, fails rather than run the subquery
unfiltered.
+ private static void applyRowFilters(PinotQuery subquery, QueryContext
queryContext) {
+ Map<String, String> queryOptions = queryContext.getQueryOptions();
+ if (queryOptions.isEmpty()) {
+ return;
+ }
+ Map<String, String> rowFilters = RlsUtils.extractRlsFilters(queryOptions);
+ if (rowFilters.isEmpty()) {
+ return;
+ }
+ String tableName = queryContext.getTableName();
+ Preconditions.checkState(tableName != null, "Row-level security filters %s
are set, but the query has no table",
+ rowFilters.keySet());
+ String rowFilterKey =
RlsUtils.buildRlsFilterKey(TableNameBuilder.extractRawTableName(tableName));
+ String rowFilter = rowFilters.get(rowFilterKey);
+ Preconditions.checkState(!StringUtils.isEmpty(rowFilter),
Review Comment:
Added in c9ccc5dfb9: with only `rlsFilters-otherTable` stamped, the query
now fails with the "none for the queried table" message, asserted in
`testInPartitionedSubqueryAppliesRowFilters`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -561,6 +560,8 @@ protected BrokerResponse doHandleRequest(long requestId,
String query, SqlNodeAn
TableRowColAccessResult rlsFilters =
accessControl.getRowColFilters(requesterIdentity, tableName);
List<String> rowFilters = rlsFilters.getRLSFilters().orElse(null);
if (rowFilters != null && !rowFilters.isEmpty()) {
+ validateCrossTableFunctionsUnderRowFilters(requestId, query,
requestContext, tableName, rawTableName,
Review Comment:
Narrowed in c9ccc5dfb9: the doc of the check now says it covers the
physical-table path only, because the logical-table path applies no row filters
at all today (neither to the WHERE clause nor here), and the PR description
carries the same scope note. That pre-existing gap is tracked separately as a
follow-up.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
