vvivekiyer commented on code in PR #10534:
URL: https://github.com/apache/pinot/pull/10534#discussion_r1157562793
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -221,6 +242,11 @@ private BrokerResponse handleRequest(long requestId,
String query,
return brokerResponse;
}
+ private boolean hasTableAccess(RequesterIdentity requesterIdentity, RelNode
relRoot) {
+ Set<String> tableNames = new
HashSet<>(RelOptUtil.findAllTableQualifiedNames(relRoot));
+ return _accessControlFactory.create().hasAccess(requesterIdentity,
tableNames);
+ }
Review Comment:
> Later in the code i see that the QueryPlan has the list of tables:
I did see this way. But didn't go with this approach because it wouldn't
work for explain plan queries.
> Optionally do you think we should push down this validation to the actual
explainPlan or planQuery methods themselves instead if you really need access
to the RelRoot to get the table names?
Yes, this is an option (and the only one available now I guess). But the
cons is that we'll have to pass a bunch of context like _accessControlFactory,
_brokerMetrics, etc to QueryContext.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -132,6 +134,13 @@ public QueryEnvironment(TypeFactory typeFactory,
CalciteSchema rootSchema, Worke
_hepProgram = hepProgramBuilder.build();
}
+ /**
+ * Returns the RelRoot generated after compiling the query.
+ */
+ public RelRoot getRelRoot() {
+ return _relRoot;
+ }
Review Comment:
Good point. This wouldn't work. Let me think of an alternate way.
--
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]
