ankitsultana commented on code in PR #16003:
URL: https://github.com/apache/pinot/pull/16003#discussion_r2129338250
##########
pinot-timeseries/pinot-timeseries-planner/src/main/java/org/apache/pinot/tsdb/planner/physical/TableScanVisitor.java:
##########
@@ -68,6 +77,53 @@ public void assignSegmentsToPlan(BaseTimeSeriesPlanNode
planNode, TimeBuckets ti
}
}
+ /**
+ * Adds table type information (offline/realtime) to the plan node.
+ * If the plan node is a leaf node, it retrieves the table route info and
updates the table name with type.
+ * If the plan node has child nodes, it recursively processes each child
node.
+ *
+ * @param planNode The {@link BaseTimeSeriesPlanNode} to process.
+ * @return The updated {@link BaseTimeSeriesPlanNode} with table type
information.
+ */
+ public BaseTimeSeriesPlanNode addTableTypeInfoToPlan(BaseTimeSeriesPlanNode
planNode) {
+ if (planNode instanceof LeafTimeSeriesPlanNode) {
+ LeafTimeSeriesPlanNode sfpNode = (LeafTimeSeriesPlanNode) planNode;
+ TableRouteInfo routeInfo =
_tableRouteProvider.getTableRouteInfo(sfpNode.getTableName(), _tableCache,
+ _routingManager);
+ String tableNameWithType = getTableNameWithType(routeInfo);
+ Preconditions.checkNotNull(tableNameWithType, "Table not found for table
name: " + sfpNode.getTableName());
Review Comment:
This is masking the true error. Can you throw in `getTableNameWithType`
directly?
##########
pinot-timeseries/pinot-timeseries-planner/src/main/java/org/apache/pinot/tsdb/planner/physical/TableScanVisitor.java:
##########
@@ -68,6 +77,53 @@ public void assignSegmentsToPlan(BaseTimeSeriesPlanNode
planNode, TimeBuckets ti
}
}
+ /**
+ * Adds table type information (offline/realtime) to the plan node.
+ * If the plan node is a leaf node, it retrieves the table route info and
updates the table name with type.
+ * If the plan node has child nodes, it recursively processes each child
node.
+ *
+ * @param planNode The {@link BaseTimeSeriesPlanNode} to process.
+ * @return The updated {@link BaseTimeSeriesPlanNode} with table type
information.
+ */
+ public BaseTimeSeriesPlanNode addTableTypeInfoToPlan(BaseTimeSeriesPlanNode
planNode) {
+ if (planNode instanceof LeafTimeSeriesPlanNode) {
+ LeafTimeSeriesPlanNode sfpNode = (LeafTimeSeriesPlanNode) planNode;
+ TableRouteInfo routeInfo =
_tableRouteProvider.getTableRouteInfo(sfpNode.getTableName(), _tableCache,
Review Comment:
Discussed offline. To support hybrid tables we'll need more changes because
we'll need to split the server requests into multiple requests. We can pick
that up later. This is good for now.
##########
pinot-timeseries/pinot-timeseries-planner/src/main/java/org/apache/pinot/tsdb/planner/TimeSeriesQueryEnvironment.java:
##########
@@ -75,7 +76,8 @@ public void init(PinotConfiguration config) {
throw new RuntimeException("Failed to instantiate logical planner for
language: " + language, e);
}
}
- TableScanVisitor.INSTANCE.init(_routingManager);
+ // TODO: Add support for logical tables in the future.
Review Comment:
nit: rename todo to `TODO(timeseries)` so we can grep and track it easily in
the future
--
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]
