[IoTDB-468] Restructure QueryPlan by Alima777 · Pull Request #796 · apache/iotdb

Alima777 · 2020-02-12T06:37:28Z

The AlignByDevice query ( which is called groupByDevice before ) and general query aligning by time are both storaged in QueryPlan now. However, they invoke different query process bascially, so it's necessary to restructure the QueryPlan and seperate the AlignByDeviceQuery from it.

The QueryPlan is designed as base class, which has RawDataQueryPlan and AlignByDevicePlan as subclasses. This restructure may bring some usage changes, because the general query which uses QueryPlan directly before has to instantiate RawDataQueryPlan now.

If you think the design is not friendly to users, welcome to comment.

qiaojialin

The QueryPlan looks nice now.

qiaojialin · 2020-02-12T07:29:10Z

+
+public class AlignByDevicePlan extends QueryPlan {
+
+  private List<String> measurements; // for group by device sql, e.g. temperature


Suggested change

private List<String> measurements; // for group by device sql, e.g. temperature

private List<String> measurements; // e.g. temperature

give a more complex example, such as m1, m2, m3

What's the difference between this measurements with the paths in QueryPlan?
Refactor the organization or rename this field

Actually, the paths in QueryPlan are paths of all devices. It's for verification and complete DataTypeMap to execute the datatypes for the execution paths this time in DataSet.

qiaojialin · 2020-02-12T07:31:26Z

+
+  private List<String> measurements; // for group by device sql, e.g. temperature
+  private Map<String, Set<String>> deviceToMeasurementsMap; // for group by device sql, e.g. root.ln.d1 -> temperature
+  private Map<String, TSDataType> dataTypeConsistencyChecker; // for group by device sql, e.g. root.ln.d1.temperature -> Float


Suggested change

private Map<String, TSDataType> dataTypeConsistencyChecker; // for group by device sql, e.g. root.ln.d1.temperature -> Float

private Map<String, TSDataType> seriesTypeMap; // e.g. root.ln.d1.temperature -> Float

make the field name clear

Add a comment to explain it.

qiaojialin · 2020-02-12T07:32:14Z

+
+  private GroupByPlan groupByPlan;
+  private FillQueryPlan fillQueryPlan;
+  private AggregationPlan aggregationPlan;


why there isn't a RawDataQueryPan

Of course there isn't a RawDataQueryPan. We don't need parameters in it.

but you need to do a RawData query for each device a

I have added a RawDataQueryPlan in AlignByDeviceDataSet.

qiaojialin · 2020-02-12T07:32:48Z

+  //the measurements that have quotation mark. e.g., "abc",
+  // '11', the data type is considered as String and the value  is considered is the same with measurement name
+  private List<String> constMeasurements = new ArrayList<>(); // for group by device sql
+  private List<Integer> positionOfConstMeasurements = new ArrayList<>(); // for group by device sql


put fields in the front of the class

qiaojialin · 2020-02-12T08:27:19Z

+    this.constMeasurements = alignByDevicePlan.getConstMeasurements();
+    this.positionOfNotExistMeasurements = alignByDevicePlan.getPositionOfNotExistMeasurements();
+    this.positionOfConstMeasurements = alignByDevicePlan.getPositionOfConstMeasurements();
    //BuildOutDataTypes();


remove this

qiaojialin · 2020-02-12T08:27:33Z

-    } else if (queryPlan instanceof FillQueryPlan) {
+    } else if (alignByDevicePlan.getFillQueryPlan() != null) {
      this.dataSetType = DataSetType.FILL;
      // assign parameters


remove this

qiaojialin · 2020-02-12T09:22:19Z

    //BuildOutDataTypes();

-    if (queryPlan instanceof GroupByPlan) {
+    if (alignByDevicePlan.getGroupByPlan() != null) {


We could add a queryType in AlignByDevicePlan, and use switch here

qiaojialin · 2020-02-12T09:25:32Z

            break;
          case QUERY:
-            QueryPlan queryPlan = new QueryPlan();
+            RawDataQueryPlan queryPlan = new RawDataQueryPlan();


Don't you save three plans in AlignByDevicePlan? They could be reused

Good Idea. Fixed.

qiaojialin · 2020-02-12T12:44:20Z

+
+  private GroupByPlan groupByPlan;
+  private FillQueryPlan fillQueryPlan;
+  private AggregationPlan aggregationPlan;


but you need to do a RawData query for each device a

qiaojialin · 2020-02-12T12:51:57Z

+    super();
+  }
+
+  public AlignByDevicePlan(boolean isQuery, Operator.OperatorType operatorType) {


remove this unused

qiaojialin · 2020-02-12T12:55:12Z


-public class GroupByPlan extends AggregationPlan {
+public class
+GroupByPlan extends AggregationPlan {


qiaojialin · 2020-02-12T12:55:55Z


-public class GroupByPlan extends AggregationPlan {
+public class
+GroupByPlan extends AggregationPlan {


Suggested change

GroupByPlan extends AggregationPlan {

GroupByTimePlan extends AggregationPlan {

Just keep it.

qiaojialin · 2020-02-12T12:56:29Z


-  private boolean isGroupByDevice = false;
-  private boolean isAlign = true;
+  private boolean isAlignByDevice = false;


Suggested change

private boolean isAlignByDevice = false;

private boolean alignByDevice = false;

Just keep it.

qiaojialin · 2020-02-12T12:56:39Z

-  private boolean isGroupByDevice = false;
-  private boolean isAlign = true;
+  private boolean isAlignByDevice = false;
+  private boolean isAlignByTime = true;


Suggested change

private boolean isAlignByTime = true;

private boolean alignByTime = true;

Just keep it please.

qiaojialin · 2020-02-12T13:00:43Z

-

 /**
 * This QueryDataSet is used for GROUP_BY_DEVICE query result.


Suggested change

* This QueryDataSet is used for GROUP_BY_DEVICE query result.

* This QueryDataSet is used for align by device query result.

qiaojialin · 2020-02-12T13:01:22Z

@@ -80,45 +77,41 @@ public class DeviceIterateDataSet extends QueryDataSet {
  private int[] currentColumnMapRelation;


how about renaming this class to AlignByDeviceDataset

It's nice. I will rename it.

qiaojialin · 2020-02-13T01:18:46Z

+        break;
+      default:
+        this.dataSetType = DataSetType.QUERY;
+        this.rawDataQueryPlan = new RawDataQueryPlan();


This logic is a little strange

Restructure QueryPlan

d84de4d

Alima777 requested a review from qiaojialin February 12, 2020 06:38

Modify isGroupByDevice to isAlignByDevice

d54050b

qiaojialin requested changes Feb 12, 2020

View reviewed changes

Fix bugs

85fe556

qiaojialin requested changes Feb 12, 2020

View reviewed changes

always endeavoring to do still better

c991383

qiaojialin reviewed Feb 13, 2020

View reviewed changes

Alima777 added 2 commits February 13, 2020 11:54

Fix a comment

749fb8f

Fix a comment

66b1187

qiaojialin approved these changes Feb 13, 2020

View reviewed changes

qiaojialin merged commit 753239a into apache:new_series_reader Feb 13, 2020


		public class AlignByDevicePlan extends QueryPlan {

		private List<String> measurements; // for group by device sql, e.g. temperature

	private List<String> measurements; // for group by device sql, e.g. temperature
	private List<String> measurements; // e.g. temperature

	private Map<String, TSDataType> dataTypeConsistencyChecker; // for group by device sql, e.g. root.ln.d1.temperature -> Float
	private Map<String, TSDataType> seriesTypeMap; // e.g. root.ln.d1.temperature -> Float

	GroupByPlan extends AggregationPlan {
	GroupByTimePlan extends AggregationPlan {

	private boolean isAlignByDevice = false;
	private boolean alignByDevice = false;

	private boolean isAlignByTime = true;
	private boolean alignByTime = true;



		/**
		* This QueryDataSet is used for GROUP_BY_DEVICE query result.

	* This QueryDataSet is used for GROUP_BY_DEVICE query result.
	* This QueryDataSet is used for align by device query result.

		@@ -80,45 +77,41 @@ public class DeviceIterateDataSet extends QueryDataSet {
		private int[] currentColumnMapRelation;

Conversation

Alima777 commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiaojialin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alima777 Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Alima777 commented Feb 12, 2020 •

edited

Loading

Alima777 Feb 12, 2020 •

edited

Loading