Jackie-Jiang commented on code in PR #17247:
URL: https://github.com/apache/pinot/pull/17247#discussion_r2684401439
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -698,6 +698,14 @@ private BrokerResponse
query(QueryEnvironment.CompiledQuery query, long requestI
}
}
+ /**
+ * Uses both the parsed SQL options and the (potentially mutated) compiled
query options to ensure we
+ * propagate everything (including queryOptions from the client) to the
servers.
+ */
+ private static Map<String, String>
mergeQueryOptions(QueryEnvironment.CompiledQuery query) {
+ return
QueryOptionsUtils.mergeQueryOptions(query.getSqlNodeAndOptions().getOptions(),
query.getOptions());
Review Comment:
I don't follow this. Both `query.getSqlNodeAndOptions().getOptions()` and
`query.getOptions()` should return the same map
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/server/ServerPlanRequestUtils.java:
##########
@@ -146,6 +146,11 @@ private static void
constructPinotQueryPlan(ServerPlanRequestContext serverConte
Map<String, String> requestMetadata) {
StagePlan stagePlan = serverContext.getStagePlan();
PinotQuery pinotQuery = serverContext.getPinotQuery();
+ if (requestMetadata != null) {
Review Comment:
`updateQueryOptions()` should be able to update query options. Do we need to
apply it here?
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java:
##########
@@ -566,6 +566,10 @@ private BrokerResponse executeSqlQuery(ObjectNode
sqlRequestJson, HttpRequesterI
SqlNodeAndOptions sqlNodeAndOptions;
try {
sqlNodeAndOptions =
RequestUtils.parseQuery(sqlRequestJson.get(Request.SQL).asText(),
sqlRequestJson);
+ if (sqlRequestJson.has(Request.QUERY_OPTIONS)) {
Review Comment:
Is this a bugfix? If so, please make a separate PR given it is out of the
scope of this PR
--
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]
