Caideyipi commented on code in PR #17205:
URL: https://github.com/apache/iotdb/pull/17205#discussion_r3360886199
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/distribute/TableDistributedPlanGenerator.java:
##########
@@ -810,6 +811,19 @@ private List<PlanNode>
constructDeviceTableScanByRegionReplicaSet(
context.deviceCrossRegion = true;
}
for (final TRegionReplicaSet regionReplicaSet : regionReplicaSets) {
+ if (analysis.isLocalQuery()) {
+ // only query this node in local query mode
+ int dataNodeId =
IoTDBDescriptor.getInstance().getConfig().getDataNodeId();
+ boolean containsThisNode =
+ regionReplicaSet.dataNodeLocations.stream()
+ .anyMatch(dn -> dn.getDataNodeId() == dataNodeId);
+ if (!containsThisNode) {
+ continue;
+ } else {
+ regionReplicaSet.dataNodeLocations.removeIf(dn ->
dn.getDataNodeId() != dataNodeId);
Review Comment:
This mutates the `TRegionReplicaSet` returned from
`DataPartition`/`PartitionCache`. Those objects are shared by the query
analysis/route cache, so after one local query the replica set can remain
truncated for later normal queries. Please build a new
`TRegionReplicaSet(regionReplicaSet.getRegionId(), localLocations)` and set/use
that instead of changing `dataNodeLocations` in place.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/distribute/TableDistributedPlanGenerator.java:
##########
@@ -810,6 +811,19 @@ private List<PlanNode>
constructDeviceTableScanByRegionReplicaSet(
context.deviceCrossRegion = true;
}
for (final TRegionReplicaSet regionReplicaSet : regionReplicaSets) {
+ if (analysis.isLocalQuery()) {
Review Comment:
This only runs in `constructDeviceTableScanByRegionReplicaSet`. When
`visitDeviceTableScan` takes the `context.isPushDownGrouping()` branch,
`constructDeviceTableScanByTags` never applies the local filter, so local
queries with grouping/tag pushdown can still schedule remote replicas. The
local restriction should be applied before both split paths or the unsupported
shapes should be rejected.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java:
##########
@@ -860,6 +860,8 @@ protected Scope visitExplainAnalyze(ExplainAnalyze node,
Optional<Scope> context
@Override
protected Scope visitQuery(Query node, Optional<Scope> context) {
analysis.setQuery(true);
+ analysis.setLocalQuery(node.getQueryBody().isLocalQuery());
Review Comment:
`Analysis` is shared across the whole statement, but `visitQuery` is also
called for subqueries/CTEs. A nested SELECT will overwrite this flag, so
`SELECT LOCALLY ... FROM (SELECT ...)` can lose local mode, and `SELECT ...
FROM (SELECT LOCALLY ...)` can make outer scans local. The local marker needs
to be scoped to the specific query/table scan or saved/restored when analyzing
nested queries.
##########
iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/RelationalSql.g4:
##########
@@ -1669,6 +1669,7 @@ LISTAGG: 'LISTAGG';
LOAD: 'LOAD';
LOADED: 'LOADED';
Review Comment:
Adding `LOCALLY` as a lexer token makes unquoted `locally` stop parsing as
an identifier unless it is also included in `nonReserved`. That breaks existing
table/column names such as `SELECT locally FROM t` or `CREATE TABLE locally
(...)`. Please keep it non-reserved if this is only a SELECT modifier.
--
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]
