Re: [PR] feat: show timeseries [order by timseries] clause [iotdb]

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

JackieTien97 commented on code in PR #17065:
URL: https://github.com/apache/iotdb/pull/17065#discussion_r2757818919


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java:
##########
@@ -2571,7 +2573,19 @@ public TSStatus updateTemplate(final TUpdateTemplateReq req) {
         ClusterTemplateManager.getInstance().commitTemplatePreSetInfo(req.getTemplateInfo());
         break;
       case UPDATE_TEMPLATE_INFO:
+        Template newTemplate =
+            TemplateInternalRPCUtil.parseUpdateTemplateInfoBytes(
+                ByteBuffer.wrap(req.getTemplateInfo()));
+        Template oldTemplate =
+            ClusterTemplateManager.getInstance().getTemplate(newTemplate.getId());
         ClusterTemplateManager.getInstance().updateTemplateInfo(req.getTemplateInfo());

Review Comment:
   pass `newTemplate` directly into ClusterTemplateManager.getInstance().updateTemplateInfo instead of parsing it again.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/operator/schema/source/TimeSeriesSchemaSource.java:
##########
@@ -54,6 +54,8 @@ public class TimeSeriesSchemaSource implements ISchemaSource<ITimeSeriesSchemaIn
   private final SchemaFilter schemaFilter;
   private final Map<Integer, Template> templateMap;
   private final boolean needViewDetail;
+  private final boolean orderByTimeseries;
+  private final boolean orderByTimeseriesDesc;

Review Comment:
   just one `private final Boolean orderByTimeseries` field is enough? and then some comments, like: null for no order by, Boolean.TRUE for asc, Boolean.FALSE for desc.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/metadata/read/TimeSeriesSchemaScanNode.java:
##########
@@ -48,6 +48,11 @@ public class TimeSeriesSchemaScanNode extends SchemaQueryScanNode {
   // if is true, the result will be sorted according to the inserting frequency of the timeseries
   private final boolean orderByHeat;
 
+  // Whether to order result by timeseries full path in this region.
+  private final boolean orderByTimeseries;
+  // When orderByTimeseries is true, whether the ordering is descending.
+  private final boolean orderByTimeseriesDesc;
+

Review Comment:
   same as above.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/planner/plan/node/process/ActiveRegionScanMergeNode.java:
##########
@@ -70,6 +70,9 @@ public PlanNode clone() {
 
   @Override
   public List<String> getOutputColumnNames() {
+    if (!children.isEmpty()) {
+      return children.get(0).getOutputColumnNames();
+    }

Review Comment:
   why we need to do this change? The origin codes should still take effect.
   ```
   ColumnHeaderConstant.showDevicesColumnHeaders.stream()
               .map(ColumnHeader::getColumnName)
               .collect(Collectors.toList())
   ```



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/schemaregion/read/req/impl/ShowTimeSeriesPlanImpl.java:
##########
@@ -37,6 +37,10 @@ public class ShowTimeSeriesPlanImpl extends AbstractShowSchemaPlanImpl
   private final SchemaFilter schemaFilter;
   private final boolean needViewDetail;
 
+  // order-by-timeseries pushdown flags inside a single SchemaRegion
+  private final boolean orderByTimeseries;
+  private final boolean orderByTimeseriesDesc;
+

Review Comment:
   same as above



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/metadata/ShowTimeSeriesStatement.java:
##########
@@ -42,6 +43,9 @@ public class ShowTimeSeriesStatement extends ShowStatement {
   // if is true, the result will be sorted according to the inserting frequency of the time series
   private final boolean orderByHeat;
   private WhereCondition timeCondition;
+  // order by timeseries name
+  private boolean orderByTimeseries;
+  private Ordering nameOrdering = Ordering.ASC;

Review Comment:
   why it's called nameOrdering? just order is enough?



-- 
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