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 2020/03/23 09:54:15 UTC
[GitHub] [incubator-iotdb] Alima777 opened a new pull request #934: Delete
dataTypeMapping etc fields in QueryPlan
Alima777 opened a new pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934
DataTypeMapping, deviceTomeasurementMap etc fields take lots of memory space while they can be replaced by other fields actually. Therefore, I delete these two fields here.
Tip: If some fields are no need to add, please use existing fields to replace them.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] samperson1997 commented on a change in pull
request #934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
samperson1997 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396971250
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -116,25 +119,29 @@ protected boolean hasNextWithoutConstraint() throws IOException {
while (deviceIterator.hasNext()) {
currentDevice = deviceIterator.next();
- Set<String> measurementColumnsOfGivenDevice = deviceToMeasurementsMap
- .get(currentDevice);
- executeColumns = new ArrayList<>(measurementColumnsOfGivenDevice);
-
- // extract paths and aggregations if exist from executeColumns
+ // get all measurements of current device
+ Set<String> measurementOfGivenDevice = currentDevice.getChildren().keySet();
+ // extract paths and aggregations queried from all measurements
+ // executeColumns is for calculating rowRecord
+ executeColumns = new ArrayList<>();
List<Path> executePaths = new ArrayList<>();
List<TSDataType> tsDataTypes = new ArrayList<>();
List<String> executeAggregations = new ArrayList<>();
- for (String column : executeColumns) {
+ for (String column : measurementDataTpeMap.keySet()) {
if (dataSetType == DataSetType.GROUPBY || dataSetType == DataSetType.AGGREGATE) {
- Path path = new Path(currentDevice,
- column.substring(column.indexOf('(') + 1, column.indexOf(')')));
- tsDataTypes.add(tsDataTypeMap.get(path));
- executePaths.add(path);
- executeAggregations.add(column.substring(0, column.indexOf('(')));
+ String measurement = column.substring(column.indexOf('(') + 1, column.indexOf(')'));
+ if (measurementOfGivenDevice.contains(measurement)){
+ executeColumns.add(column);
+ executePaths.add(new Path(currentDevice.getFullPath(), measurement));
+ tsDataTypes.add(measurementDataTpeMap.get(column));
+ executeAggregations.add(column.substring(0, column.indexOf('(')));
+ }
} else {
- Path path = new Path(currentDevice, column);
- tsDataTypes.add(tsDataTypeMap.get(path));
- executePaths.add(path);
+ if (measurementOfGivenDevice.contains(column)) {
+ executeColumns.add(column);
+ executePaths.add(new Path(currentDevice.getFullPath(), column));
+ tsDataTypes.add(measurementDataTpeMap.get(column));
+ }
Review comment:
try to extract the same process to avoid duplicated codes:
```
String measurement = column;
if (dataSetType == DataSetType.GROUPBY || dataSetType == DataSetType.AGGREGATE) {
measurement = column.substring(column.indexOf('(') + 1, column.indexOf(')'));
if (measurementOfGivenDevice.contains(measurement)) {
executeAggregations.add(column.substring(0, column.indexOf('(')));
}
}
if (measurementOfGivenDevice.contains(measurement)) {
executeColumns.add(measurement);
executePaths.add(new Path(currentDevice.getFullPath(), measurement));
tsDataTypes.add(measurementDataTpeMap.get(measurement));
}
```
(please first ensure codes above work : ) )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] qiaojialin merged pull request #934: Delete
dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
qiaojialin merged pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] Alima777 commented on a change in pull request
#934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396987520
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -116,25 +119,29 @@ protected boolean hasNextWithoutConstraint() throws IOException {
while (deviceIterator.hasNext()) {
currentDevice = deviceIterator.next();
- Set<String> measurementColumnsOfGivenDevice = deviceToMeasurementsMap
- .get(currentDevice);
- executeColumns = new ArrayList<>(measurementColumnsOfGivenDevice);
-
- // extract paths and aggregations if exist from executeColumns
+ // get all measurements of current device
+ Set<String> measurementOfGivenDevice = currentDevice.getChildren().keySet();
+ // extract paths and aggregations queried from all measurements
+ // executeColumns is for calculating rowRecord
+ executeColumns = new ArrayList<>();
List<Path> executePaths = new ArrayList<>();
List<TSDataType> tsDataTypes = new ArrayList<>();
List<String> executeAggregations = new ArrayList<>();
- for (String column : executeColumns) {
+ for (String column : measurementDataTpeMap.keySet()) {
if (dataSetType == DataSetType.GROUPBY || dataSetType == DataSetType.AGGREGATE) {
- Path path = new Path(currentDevice,
- column.substring(column.indexOf('(') + 1, column.indexOf(')')));
- tsDataTypes.add(tsDataTypeMap.get(path));
- executePaths.add(path);
- executeAggregations.add(column.substring(0, column.indexOf('(')));
+ String measurement = column.substring(column.indexOf('(') + 1, column.indexOf(')'));
+ if (measurementOfGivenDevice.contains(measurement)){
+ executeColumns.add(column);
+ executePaths.add(new Path(currentDevice.getFullPath(), measurement));
+ tsDataTypes.add(measurementDataTpeMap.get(column));
+ executeAggregations.add(column.substring(0, column.indexOf('(')));
+ }
} else {
- Path path = new Path(currentDevice, column);
- tsDataTypes.add(tsDataTypeMap.get(path));
- executePaths.add(path);
+ if (measurementOfGivenDevice.contains(column)) {
+ executeColumns.add(column);
+ executePaths.add(new Path(currentDevice.getFullPath(), column));
+ tsDataTypes.add(measurementDataTpeMap.get(column));
+ }
Review comment:
Thank you! It's very helpful for me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] samperson1997 commented on a change in pull
request #934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
samperson1997 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396969291
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -21,12 +21,16 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.iotdb.db.exception.StorageEngineException;
+import org.apache.iotdb.db.exception.metadata.MetadataException;
import org.apache.iotdb.db.exception.query.QueryProcessException;
+import org.apache.iotdb.db.metadata.MManager;
Review comment:
Remove some useless imports
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] Alima777 commented on a change in pull request
#934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396985396
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/MManager.java
##########
@@ -548,13 +548,13 @@ public TSDataType getSeriesType(String path) throws MetadataException {
/**
- * Get all devices under given prefixPath.
+ * Get all device nodes under given prefixPath.
*
* @param prefixPath a prefix of a full path. if the wildcard is not at the tail, then each
* wildcard can only match one level, otherwise it can match to the tail.
- * @return A HashSet instance which stores devices names with given prefixPath.
+ * @return A list which stores device nodes with given prefixPath.
*/
- public Set<String> getDevices(String prefixPath) throws MetadataException {
+ public List<MNode> getDevices(String prefixPath) throws MetadataException {
Review comment:
If using `String` here, we have to find the node again by path string in AlignByDeviceDataSet. So storaging MNode directly here can reduce that process.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] Alima777 commented on a change in pull request
#934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396986575
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/AlignByDeviceDataSet.java
##########
@@ -21,12 +21,16 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.iotdb.db.exception.StorageEngineException;
+import org.apache.iotdb.db.exception.metadata.MetadataException;
import org.apache.iotdb.db.exception.query.QueryProcessException;
+import org.apache.iotdb.db.metadata.MManager;
Review comment:
Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] samperson1997 commented on a change in pull
request #934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
samperson1997 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396989971
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/MManager.java
##########
@@ -548,13 +548,13 @@ public TSDataType getSeriesType(String path) throws MetadataException {
/**
- * Get all devices under given prefixPath.
+ * Get all device nodes under given prefixPath.
*
* @param prefixPath a prefix of a full path. if the wildcard is not at the tail, then each
* wildcard can only match one level, otherwise it can match to the tail.
- * @return A HashSet instance which stores devices names with given prefixPath.
+ * @return A list which stores device nodes with given prefixPath.
*/
- public Set<String> getDevices(String prefixPath) throws MetadataException {
+ public List<MNode> getDevices(String prefixPath) throws MetadataException {
Review comment:
Sure. I didn't figure out a better way either... Maybe we could keep this then : )
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] samperson1997 commented on a change in pull
request #934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
samperson1997 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r396967704
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/MManager.java
##########
@@ -548,13 +548,13 @@ public TSDataType getSeriesType(String path) throws MetadataException {
/**
- * Get all devices under given prefixPath.
+ * Get all device nodes under given prefixPath.
*
* @param prefixPath a prefix of a full path. if the wildcard is not at the tail, then each
* wildcard can only match one level, otherwise it can match to the tail.
- * @return A HashSet instance which stores devices names with given prefixPath.
+ * @return A list which stores device nodes with given prefixPath.
*/
- public Set<String> getDevices(String prefixPath) throws MetadataException {
+ public List<MNode> getDevices(String prefixPath) throws MetadataException {
Review comment:
Actually I think it's better to use `String` here so that you won't use `getFullPath()` when call this method ... What do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-iotdb] Alima777 commented on a change in pull request
#934: Delete dataTypeMapping etc fields in QueryPlan
Posted by GitBox <gi...@apache.org>.
Alima777 commented on a change in pull request #934: Delete dataTypeMapping etc fields in QueryPlan
URL: https://github.com/apache/incubator-iotdb/pull/934#discussion_r397006038
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/MManager.java
##########
@@ -548,13 +548,13 @@ public TSDataType getSeriesType(String path) throws MetadataException {
/**
- * Get all devices under given prefixPath.
+ * Get all device nodes under given prefixPath.
*
* @param prefixPath a prefix of a full path. if the wildcard is not at the tail, then each
* wildcard can only match one level, otherwise it can match to the tail.
- * @return A HashSet instance which stores devices names with given prefixPath.
+ * @return A list which stores device nodes with given prefixPath.
*/
- public Set<String> getDevices(String prefixPath) throws MetadataException {
+ public List<MNode> getDevices(String prefixPath) throws MetadataException {
Review comment:
Actually It doesn't cost that much time finding device node. Like what you said, to keep the consistence with other methods, I deciced to change it to return List<String>.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services