vrajat commented on code in PR #15773:
URL: https://github.com/apache/pinot/pull/15773#discussion_r2099458411
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/table/LogicalTableRouteInfo.java:
##########
@@ -32,22 +32,23 @@
import org.apache.pinot.core.routing.ServerRouteInfo;
import org.apache.pinot.core.routing.TimeBoundaryInfo;
import org.apache.pinot.core.transport.BaseTableRouteInfo;
+import org.apache.pinot.core.transport.ImplicitHybridTableRouteInfo;
import org.apache.pinot.core.transport.ServerInstance;
import org.apache.pinot.core.transport.ServerRoutingInstance;
import org.apache.pinot.core.transport.TableRouteInfo;
+import org.apache.pinot.query.timeboundary.TimeBoundaryStrategy;
import org.apache.pinot.spi.config.table.QueryConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
-import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.query.QueryThreadContext;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
public class LogicalTableRouteInfo extends BaseTableRouteInfo {
- private final LogicalTableConfig _logicalTable;
- private List<TableRouteInfo> _offlineTables;
- private List<TableRouteInfo> _realtimeTables;
+ private String _logicalTableName;
+ private List<ImplicitHybridTableRouteInfo> _offlineTables;
Review Comment:
I had tried that. IMO it didnt add much value. Specifically,
`ImplicitHybridTableRouteInfo` has the following structure:
```
if (hasOffline()) {
...
}
if (hasRealtime()) {
...
}
if (hasBoth()) {
... do some extra work ...
}
```
A `PhysicalTableRouteInfo` & `PhysicalTableRouteInfo` will have:
```
if (isOffline()) {
...
} else if (isRealtime()) {
...
}
```
One if condition is saved. However the code for `ImplicitHybridTable` and
`PhysicalHybridTable` is exactly the same. So there will be
* duplicate code
OR
* `ImplicitHybridTable` should consist of 2 `PhysicalHybridTable`.
The second option will make it hard to guarantee backward compatibility
because I cannot copy/paste code from `BaseSingleStageRequestHandler` to
`ImplicitHybridTableRouteProvider`.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
