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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
