yashmayya commented on code in PR #16043:
URL: https://github.com/apache/pinot/pull/16043#discussion_r2160764540
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -304,6 +304,17 @@ private void checkAuthorization(RequesterIdentity
requesterIdentity, RequestCont
if (!tableAuthorizationResult.hasAccess()) {
throwTableAccessError(tableAuthorizationResult);
}
+ AccessControl accessControl = _accessControlFactory.create();
+ for (String tableName : tables) {
+ accessControl.getRowColFilters(requesterIdentity,
tableName).getRLSFilters()
+ .ifPresent(rowFilters -> {
+ String combinedFilters = String.join(" AND ", rowFilters);
+ String key = String.format("%s-%s", CommonConstants.RLS_FILTERS,
tableName);
Review Comment:
Yeah I'm not a huge fan of this approach either tbh, but I'm not sure if
there's a cleaner way to pass on this context from the broker all the way to
the leaf stage compilation in the servers without introducing a bunch of
additional complexity. @gortiz any idea if there's an equally simple but
cleaner way to do this?
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -434,6 +436,20 @@ protected BrokerResponse doHandleRequest(long requestId,
String query, SqlNodeAn
throwAccessDeniedError(requestId, query, requestContext, tableName,
authorizationResult);
}
+ TableRowColAuthResult rlsFilters =
accessControl.getRowColFilters(requesterIdentity, tableName);
+
+ //rewrite query
+ Map<String, String> queryOptions =
+ pinotQuery.getQueryOptions() == null ? new HashMap<>() :
pinotQuery.getQueryOptions();
+
+ rlsFilters.getRLSFilters().ifPresent(rowFilters -> {
+ String combinedFilters =
+ rowFilters.stream().map(filter -> "( " + filter + "
)").collect(Collectors.joining(" AND "));
+ queryOptions.put(tableName, combinedFilters);
+ pinotQuery.setQueryOptions(queryOptions);
+ CalciteSqlParser.queryRewrite(pinotQuery);
Review Comment:
Why are we applying all the query rewriters multiple times here?
--
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]
