xiangfu0 commented on code in PR #20169:
URL: https://github.com/apache/pinot/pull/20169#discussion_r4498645354
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -561,6 +567,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()) {
+ rejectCrossTableFunctionsUnderRowFilters(requestId, query,
requestContext, tableName, rawTableName,
Review Comment:
Inlined in 47f4cedd64: the check now sits in the nonempty-row-filters block
with a short comment; the helper and its Javadoc are gone.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -1179,6 +1187,12 @@ private CompileResult compileRequest(long requestId,
String query, SqlNodeAndOpt
return new CompileResult(new
BrokerResponseNative(QueryErrorCode.QUERY_VALIDATION, "Table name not found"));
}
+ // Collected before handleSubquery() replaces each IN_SUBQUERY call with
IN_ID_SET on its result, for
+ // doHandleRequest to reject cross-table functions once it knows whether
row filters apply. For a gapfill query
+ // the two differ, and only the stripped one reaches a server.
+ boolean hasCrossTableFunction = hasCrossTableFunction(serverPinotQuery)
Review Comment:
Done in 47f4cedd64: the scan is gated on `_enableRowColumnLevelAuth`, still
captured before `handleSubquery()` removes the `IN_SUBQUERY` calls.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java:
##########
@@ -537,16 +538,18 @@ private void handleSubquery(FilterContext filter,
TableExecutionInfo executionIn
/// rewrites the expression with the subquery response.
///
/// Currently only supports ID_SET subquery within the
IN_PARTITIONED_SUBQUERY transform function, which will be
- /// rewritten to an IN_ID_SET transform function.
+ /// rewritten to an IN_ID_SET transform function. The function is not
allowed under row-level security: the broker
+ /// rejects it, and a server behind an older broker fails the query, see
[#checkNoRowFilters(QueryContext)].
private void handleSubquery(ExpressionContext expression, TableExecutionInfo
executionInfo, TimerContext timerContext,
- ExecutorService executorService, long endTimeMs)
+ ExecutorService executorService, QueryContext queryContext)
throws Exception {
FunctionContext function = expression.getFunction();
if (function == null) {
return;
}
List<ExpressionContext> arguments = function.getArguments();
if (StringUtils.remove(function.getFunctionName(),
'_').equalsIgnoreCase(IN_PARTITIONED_SUBQUERY)) {
+ checkNoRowFilters(queryContext);
Review Comment:
Inlined in 47f4cedd64 at the start of the `IN_PARTITIONED_SUBQUERY` branch,
with a short comment; the helper and its Javadoc are gone.
_🤖 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]
