Re: [PR] Support select locally for data consistency check [iotdb]

["Caideyipi (via GitHub)" <gi...@apache.org>]

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: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org