You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/12/06 03:16:57 UTC
[GitHub] [iotdb] JackieTien97 commented on a diff in pull request #8082: [IOTDB-4572] [IOTDB-3580] support order by in align by device
JackieTien97 commented on code in PR #8082:
URL: https://github.com/apache/iotdb/pull/8082#discussion_r1039384657
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
Review Comment:
```suggestion
@RunWith(IoTDBTestRunner.class)
@Category({LocalStandaloneIT.class, ClusterIT.class})
```
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
+public class IoTDBOrderByWithAlignByDeviceIT {
+ private static final String[] places =
+ new String[] {
+ "root.weather.London",
+ "root.weather.Edinburgh",
+ "root.weather.Belfast",
+ "root.weather.Birmingham",
+ "root.weather.Liverpool",
+ "root.weather.Derby",
+ "root.weather.Durham",
+ "root.weather.Hereford",
+ "root.weather.Manchester",
+ "root.weather.Oxford"
+ };
+ private static final long startPrecipitation = 200;
+ private static final double startTemperature = 20.0;
+ private static final long startTime = 1668960000000L;
+ private static final int numOfTSInDevice = 20;
+ private static final long timeGap = 100L;
+ private static final Map<String, Long> deviceToStartTimestamp = new HashMap<>();
+
+ public static final Map<String, Double[]> deviceToMaxTemperature = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgTemperature = new HashMap<>();
+ public static final Map<String, Long[]> deviceToMaxPrecipitation = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgPrecipitation = new HashMap<>();
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ EnvFactory.getEnv().initBeforeClass();
+ insertData();
+ }
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ EnvFactory.getEnv().cleanAfterClass();
+ }
+
+ private static void insertData() {
+ try (Connection iotDBConnection = EnvFactory.getEnv().getConnection();
+ Statement statement = iotDBConnection.createStatement()) {
+ // create TimeSeries
+ for (String place : places) {
+ String PRE_PRECIPITATION = place + ".precipitation";
+ String PRE_TEMPERATURE = place + ".temperature";
+ String createPrecipitationSql =
+ "CREATE TIMESERIES " + PRE_PRECIPITATION + " WITH DATATYPE=INT64, ENCODING=RLE";
+ String createTemperatureSql =
+ "CREATE TIMESERIES " + PRE_TEMPERATURE + " WITH DATATYPE=DOUBLE, ENCODING=RLE";
+ statement.execute(createPrecipitationSql);
+ statement.execute(createTemperatureSql);
+ }
+ // insert data
+ long start = startTime;
+ double[][] temperatures = new double[places.length][29];
+ long[][] precipitations = new long[places.length][29];
Review Comment:
29 or 20?
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
+public class IoTDBOrderByWithAlignByDeviceIT {
+ private static final String[] places =
+ new String[] {
+ "root.weather.London",
+ "root.weather.Edinburgh",
+ "root.weather.Belfast",
+ "root.weather.Birmingham",
+ "root.weather.Liverpool",
+ "root.weather.Derby",
+ "root.weather.Durham",
+ "root.weather.Hereford",
+ "root.weather.Manchester",
+ "root.weather.Oxford"
+ };
+ private static final long startPrecipitation = 200;
+ private static final double startTemperature = 20.0;
+ private static final long startTime = 1668960000000L;
+ private static final int numOfTSInDevice = 20;
+ private static final long timeGap = 100L;
+ private static final Map<String, Long> deviceToStartTimestamp = new HashMap<>();
+
+ public static final Map<String, Double[]> deviceToMaxTemperature = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgTemperature = new HashMap<>();
+ public static final Map<String, Long[]> deviceToMaxPrecipitation = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgPrecipitation = new HashMap<>();
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ EnvFactory.getEnv().initBeforeClass();
+ insertData();
+ }
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ EnvFactory.getEnv().cleanAfterClass();
+ }
+
+ private static void insertData() {
+ try (Connection iotDBConnection = EnvFactory.getEnv().getConnection();
+ Statement statement = iotDBConnection.createStatement()) {
+ // create TimeSeries
+ for (String place : places) {
+ String PRE_PRECIPITATION = place + ".precipitation";
+ String PRE_TEMPERATURE = place + ".temperature";
+ String createPrecipitationSql =
+ "CREATE TIMESERIES " + PRE_PRECIPITATION + " WITH DATATYPE=INT64, ENCODING=RLE";
+ String createTemperatureSql =
+ "CREATE TIMESERIES " + PRE_TEMPERATURE + " WITH DATATYPE=DOUBLE, ENCODING=RLE";
+ statement.execute(createPrecipitationSql);
+ statement.execute(createTemperatureSql);
+ }
+ // insert data
+ long start = startTime;
+ double[][] temperatures = new double[places.length][29];
+ long[][] precipitations = new long[places.length][29];
+ for (int index = 0; index < places.length; index++) {
+ String place = places[index];
+
+ for (int i = 0; i < numOfTSInDevice; i++) {
+ long precipitation = startPrecipitation + place.hashCode() + (start + i * timeGap);
+ double temperature = startTemperature + place.hashCode() + (start + i * timeGap);
+ precipitations[index][(int) ((start - startTime) / timeGap) + i] = precipitation;
Review Comment:
`i` or `((start - startTime) / timeGap) + i`?
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/process/SingleDeviceViewNode.java:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.mpp.plan.planner.plan.node.process;
+
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanNode;
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanNodeId;
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanNodeType;
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanVisitor;
+import org.apache.iotdb.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+public class SingleDeviceViewNode extends SingleChildProcessNode {
+
+ private final String device;
+ private final List<String> outputColumnNames;
Review Comment:
avoid putting this field in each `SingleDeviceViewNode`
##########
server/src/main/java/org/apache/iotdb/db/utils/MergeSortUtils.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.utils;
+
+import org.apache.iotdb.db.mpp.plan.statement.component.Ordering;
+import org.apache.iotdb.tsfile.read.common.block.TsBlock;
+import org.apache.iotdb.tsfile.read.common.block.TsBlock.TsBlockSingleColumnIterator;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+public abstract class MergeSortUtils {
Review Comment:
change it according to last night's discussion.
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/OperatorTreeGenerator.java:
##########
@@ -639,6 +645,24 @@ public Operator visitNodePathsCount(NodePathsCountNode node, LocalExecutionPlanC
return new NodePathsCountOperator(operatorContext, child);
}
+ @Override
+ public Operator visitSingleDeviceView(
+ SingleDeviceViewNode node, LocalExecutionPlanContext context) {
+ OperatorContext operatorContext =
+ context
+ .getInstanceContext()
+ .addOperatorContext(
+ context.getNextOperatorId(),
+ node.getPlanNodeId(),
+ SingleDeviceViewOperator.class.getSimpleName());
+ Operator child = node.getChild().accept(this, context);
+ List<Integer> deviceColumnIndex = node.getDeviceToMeasurementIndexes();
+ List<TSDataType> outputColumnTypes = getOutputColumnTypes(node, context.getTypeProvider());
Review Comment:
This will cause each `SingleDeviceView` create a new `List<TsDataType>`, you should put `List<TsDataType>` in `LocalExecutionPlanContext` while visiting MergeSortNode and then reuse that in each `SingleDeviceView`
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
Review Comment:
Without these annotations, your added ITs won't be run.
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
+public class IoTDBOrderByWithAlignByDeviceIT {
+ private static final String[] places =
+ new String[] {
+ "root.weather.London",
+ "root.weather.Edinburgh",
+ "root.weather.Belfast",
+ "root.weather.Birmingham",
+ "root.weather.Liverpool",
+ "root.weather.Derby",
+ "root.weather.Durham",
+ "root.weather.Hereford",
+ "root.weather.Manchester",
+ "root.weather.Oxford"
+ };
+ private static final long startPrecipitation = 200;
+ private static final double startTemperature = 20.0;
+ private static final long startTime = 1668960000000L;
+ private static final int numOfTSInDevice = 20;
+ private static final long timeGap = 100L;
+ private static final Map<String, Long> deviceToStartTimestamp = new HashMap<>();
+
+ public static final Map<String, Double[]> deviceToMaxTemperature = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgTemperature = new HashMap<>();
+ public static final Map<String, Long[]> deviceToMaxPrecipitation = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgPrecipitation = new HashMap<>();
Review Comment:
check all your changed files about the primitive type array.
##########
server/src/main/java/org/apache/iotdb/db/utils/DeviceMergeUtils.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.utils;
+
+import org.apache.iotdb.db.mpp.plan.statement.component.Ordering;
+import org.apache.iotdb.db.mpp.plan.statement.component.SortItem;
+import org.apache.iotdb.tsfile.read.common.block.TsBlock;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class DeviceMergeUtils extends MergeSortUtils {
+
+ String[] startKey;
+ String[] endKey;
+
+ public DeviceMergeUtils(List<SortItem> sortItemList, int childNum) {
+ this.deviceOrdering = sortItemList.get(0).getOrdering();
+ this.timeOrdering = sortItemList.get(1).getOrdering();
+ this.tsBlockCount = childNum;
+ this.startKey = new String[tsBlockCount];
+ this.endKey = new String[tsBlockCount];
+ this.tsBlocksExist = new boolean[tsBlockCount];
+ this.tsBlocks = new TsBlock[tsBlockCount];
+ this.keyValueSelector = new KeyValueSelector(tsBlockCount);
+ }
+
+ @Override
+ public void addTsBlock(TsBlock tsBlock, int index) {
+ this.tsBlocks[index] = tsBlock;
+ startKey[index] = tsBlock.getColumn(0).getBinary(0).toString();
+ endKey[index] = startKey[index];
+ tsBlocksExist[index] = true;
+ }
+
+ @Override
+ public void updateTsBlock(int index, TsBlock tsBlock) {
+ if (tsBlock == null) {
+ tsBlocks[index] = null;
+ tsBlocksExist[index] = false;
+ } else {
Review Comment:
Should never use nullness judgement to do different things. It's better to add a new method like `reset(int index)`.
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/PlanGraphPrinter.java:
##########
@@ -152,6 +162,21 @@ public List<String> visitDeviceView(DeviceViewNode node, GraphContext context) {
return render(node, boxValue, context);
}
+ @Override
+ public List<String> visitMergeSort(MergeSortNode node, GraphContext context) {
+ List<String> boxValue = new ArrayList<>();
+ boxValue.add(String.format("MergeSort-%s", node.getPlanNodeId().getId()));
+ boxValue.add(String.format("ChildrenCount: %d", node.getChildren().size()));
+ boxValue.add(
+ String.format(
+ "Order: %s %s,%s %s",
+ node.getMergeOrderParameter().getSortItemList().get(0).getSortKey(),
+ node.getMergeOrderParameter().getSortItemList().get(0).getOrdering(),
+ node.getMergeOrderParameter().getSortItemList().get(1).getSortKey(),
+ node.getMergeOrderParameter().getSortItemList().get(1).getOrdering()));
Review Comment:
may not only contain these two, try to print all sort items.
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
+public class IoTDBOrderByWithAlignByDeviceIT {
+ private static final String[] places =
+ new String[] {
+ "root.weather.London",
+ "root.weather.Edinburgh",
+ "root.weather.Belfast",
+ "root.weather.Birmingham",
+ "root.weather.Liverpool",
+ "root.weather.Derby",
+ "root.weather.Durham",
+ "root.weather.Hereford",
+ "root.weather.Manchester",
+ "root.weather.Oxford"
+ };
+ private static final long startPrecipitation = 200;
+ private static final double startTemperature = 20.0;
+ private static final long startTime = 1668960000000L;
+ private static final int numOfTSInDevice = 20;
+ private static final long timeGap = 100L;
+ private static final Map<String, Long> deviceToStartTimestamp = new HashMap<>();
+
+ public static final Map<String, Double[]> deviceToMaxTemperature = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgTemperature = new HashMap<>();
+ public static final Map<String, Long[]> deviceToMaxPrecipitation = new HashMap<>();
+ public static final Map<String, Double[]> deviceToAvgPrecipitation = new HashMap<>();
Review Comment:
```suggestion
public static final Map<String, double[]> deviceToMaxTemperature = new HashMap<>();
public static final Map<String, double[]> deviceToAvgTemperature = new HashMap<>();
public static final Map<String, long[]> deviceToMaxPrecipitation = new HashMap<>();
public static final Map<String, double[]> deviceToAvgPrecipitation = new HashMap<>();
```
##########
server/src/main/java/org/apache/iotdb/db/utils/DeviceMergeUtils.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.utils;
+
+import org.apache.iotdb.db.mpp.plan.statement.component.Ordering;
+import org.apache.iotdb.db.mpp.plan.statement.component.SortItem;
+import org.apache.iotdb.tsfile.read.common.block.TsBlock;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class DeviceMergeUtils extends MergeSortUtils {
Review Comment:
delete.
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/process/MergeSortNode.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.mpp.plan.planner.plan.node.process;
+
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanNode;
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanNodeId;
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanNodeType;
+import org.apache.iotdb.db.mpp.plan.planner.plan.node.PlanVisitor;
+import org.apache.iotdb.db.mpp.plan.planner.plan.parameter.OrderByParameter;
+import org.apache.iotdb.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+public class MergeSortNode extends MultiChildProcessNode {
+
+ private final OrderByParameter mergeOrderParameter;
+
+ private final List<String> devices;
Review Comment:
this field should not appear in `MergeSortNode`.
##########
integration-test/src/test/java/org/apache/iotdb/db/it/alignbydevice/IoTDBOrderByWithAlignByDeviceIT.java:
##########
@@ -0,0 +1,1201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.it.alignbydevice;
+
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.ClusterIT;
+import org.apache.iotdb.itbase.category.LocalStandaloneIT;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+@RunWith(IoTDBTestRunner.class)
+public class IoTDBOrderByWithAlignByDeviceIT {
Review Comment:
Add a google doc for this IT to describe the data it generates, like the following:
```
/**
* This class generates data for test cases in aligned time series scenarios.
*
* <p>You can comprehensively view the generated data in the following online doc:
*
* <p>https://docs.google.com/spreadsheets/d/1kfrSR1_paSd9B1Z0jnPBD3WQIMDslDuNm4R0mpWx9Ms/edit?usp=sharing
*/
public class AlignedWriteUtil {
```
##########
server/src/main/java/org/apache/iotdb/db/utils/DeviceMergeUtils.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.utils;
+
+import org.apache.iotdb.db.mpp.plan.statement.component.Ordering;
+import org.apache.iotdb.db.mpp.plan.statement.component.SortItem;
+import org.apache.iotdb.tsfile.read.common.block.TsBlock;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class DeviceMergeUtils extends MergeSortUtils {
+
+ String[] startKey;
+ String[] endKey;
+
+ public DeviceMergeUtils(List<SortItem> sortItemList, int childNum) {
+ this.deviceOrdering = sortItemList.get(0).getOrdering();
+ this.timeOrdering = sortItemList.get(1).getOrdering();
+ this.tsBlockCount = childNum;
+ this.startKey = new String[tsBlockCount];
+ this.endKey = new String[tsBlockCount];
+ this.tsBlocksExist = new boolean[tsBlockCount];
+ this.tsBlocks = new TsBlock[tsBlockCount];
+ this.keyValueSelector = new KeyValueSelector(tsBlockCount);
+ }
+
+ @Override
+ public void addTsBlock(TsBlock tsBlock, int index) {
+ this.tsBlocks[index] = tsBlock;
+ startKey[index] = tsBlock.getColumn(0).getBinary(0).toString();
+ endKey[index] = startKey[index];
+ tsBlocksExist[index] = true;
+ }
+
+ @Override
+ public void updateTsBlock(int index, TsBlock tsBlock) {
+ if (tsBlock == null) {
+ tsBlocks[index] = null;
+ tsBlocksExist[index] = false;
+ } else {
+ tsBlocks[index] = tsBlock;
+ startKey[index] = tsBlocks[index].getColumn(0).getBinary(0).toString();
Review Comment:
each time call toString() is not effective, actually `Binary` class has already implemented `Comparable<Binary>`
##########
server/src/main/java/org/apache/iotdb/db/utils/TimeMergeUtils.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.utils;
+
+import org.apache.iotdb.db.mpp.plan.statement.component.Ordering;
+import org.apache.iotdb.db.mpp.plan.statement.component.SortItem;
+import org.apache.iotdb.tsfile.read.common.block.TsBlock;
+
+import java.util.Collections;
+import java.util.List;
+
+public class TimeMergeUtils extends MergeSortUtils {
Review Comment:
delete
--
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