Jackie-Jiang commented on code in PR #10673:
URL: https://github.com/apache/pinot/pull/10673#discussion_r1178238278
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -237,6 +239,7 @@ protected RelRoot compileQuery(SqlNode sqlNode,
PlannerContext plannerContext)
SqlNode validated = validate(sqlNode, plannerContext);
RelRoot relation = toRelation(validated, plannerContext);
RelNode optimized = optimize(relation, plannerContext);
+ RelDecorrelator.decorrelateQuery(optimized, RelBuilder.create(_config));
Review Comment:
Is this related?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/QueryServerInstance.java:
##########
@@ -24,48 +24,42 @@
/**
- * {@code VirtualServer} is a {@link ServerInstance} associated with a
- * unique virtualization identifier which allows the multistage query
- * engine to collocate multiple virtual servers on a single physical
- * instance, enabling higher levels of parallelism and partitioning
- * the query input.
+ * {@code QueryServerInstance} is representation used during query dispatch to
indicate the
+ * physical location of a query server.
+ *
+ * <p>Note that {@code QueryServerInstance} should only be used during
dispatch.</p>
*/
-public class VirtualServer {
-
- private final ServerInstance _server;
- private final int _virtualId;
-
- public VirtualServer(ServerInstance server, int virtualId) {
- _server = server;
- _virtualId = virtualId;
+public class QueryServerInstance {
+ private final String _hostname;
+ private final int _port;
Review Comment:
What is this port? Do we need it?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/QueryPlan.java:
##########
@@ -39,13 +45,15 @@
public class QueryPlan {
private final List<Pair<Integer, String>> _queryResultFields;
private final Map<Integer, StageNode> _queryStageMap;
- private final Map<Integer, StageMetadata> _stageMetadataMap;
+ private final List<StageMetadata> _stageMetadataList;
+ private final Map<Integer, DispatchablePlanMetadata>
_dispatchablePlanMetadataMap;
Review Comment:
These maps can be simplified into List (key is always 0 to `numStages - 1`).
Do you see the need of keeping it as map long term wise?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/ExplainPlanStageVisitor.java:
##########
@@ -62,7 +63,8 @@ public static String explain(QueryPlan queryPlan) {
}
// the root of a query plan always only has a single node
- VirtualServer rootServer =
queryPlan.getStageMetadataMap().get(0).getServerInstances().get(0);
+ QueryServerInstance rootServer = new ArrayList<>(
Review Comment:
You can do `.iterator().next()` to get the only element from a set
--
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]
