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/11/07 05:23:18 UTC
[GitHub] [iotdb] DanielWang2035 opened a new pull request, #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
DanielWang2035 opened a new pull request, #7918:
URL: https://github.com/apache/iotdb/pull/7918
see https://issues.apache.org/jira/browse/IOTDB-4558
--
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
[GitHub] [iotdb] HTHou commented on a diff in pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
HTHou commented on code in PR #7918:
URL: https://github.com/apache/iotdb/pull/7918#discussion_r1018041664
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/write/InsertRowsOfOneDeviceNode.java:
##########
@@ -197,7 +199,13 @@ public List<WritePlanNode> splitByPartition(Analysis analysis) {
private void storeMeasurementsAndDataType() {
Map<String, TSDataType> measurementsAndDataType = new HashMap<>();
- for (InsertRowNode insertRowNode : insertRowNodeList) {
+ /*
+ Traverse from end to start. For example, consider this sql:
+ "insert into root.t1.wf01.wt01(timestamp, s1) values(1, 1.0), (2, 'hello')"
+ we want the type of 's1' to be inferred as FLOAT(1.0) rather than TEXT('hello').
+ */
Review Comment:
Add some IT for this case?
--
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
[GitHub] [iotdb] HTHou merged pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
HTHou merged PR #7918:
URL: https://github.com/apache/iotdb/pull/7918
--
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
[GitHub] [iotdb] DanielWang2035 commented on a diff in pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
DanielWang2035 commented on code in PR #7918:
URL: https://github.com/apache/iotdb/pull/7918#discussion_r1015136023
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/write/InsertRowsOfOneDeviceNode.java:
##########
@@ -153,7 +153,9 @@ public void validateAndSetSchema(ISchemaTree schemaTree)
this.failedMeasurementIndex2Info = insertRowNode.failedMeasurementIndex2Info;
}
}
- storeMeasurementsAndDataType();
+ if (!this.hasFailedMeasurements()) {
+ storeMeasurementsAndDataType();
Review Comment:
I don't know why it is called here.
--
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
[GitHub] [iotdb] HTHou commented on a diff in pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
HTHou commented on code in PR #7918:
URL: https://github.com/apache/iotdb/pull/7918#discussion_r1018048080
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/write/InsertRowsOfOneDeviceNode.java:
##########
@@ -153,7 +153,9 @@ public void validateAndSetSchema(ISchemaTree schemaTree)
this.failedMeasurementIndex2Info = insertRowNode.failedMeasurementIndex2Info;
}
}
- storeMeasurementsAndDataType();
+ if (!this.hasFailedMeasurements()) {
+ storeMeasurementsAndDataType();
Review Comment:
If we don't fill datatypes in AnalyzeVisitor, the datatypes should be stored here?
--
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
[GitHub] [iotdb] DanielWang2035 commented on a diff in pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
DanielWang2035 commented on code in PR #7918:
URL: https://github.com/apache/iotdb/pull/7918#discussion_r1018552447
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/node/write/InsertRowsOfOneDeviceNode.java:
##########
@@ -197,7 +199,13 @@ public List<WritePlanNode> splitByPartition(Analysis analysis) {
private void storeMeasurementsAndDataType() {
Map<String, TSDataType> measurementsAndDataType = new HashMap<>();
- for (InsertRowNode insertRowNode : insertRowNodeList) {
+ /*
+ Traverse from end to start. For example, consider this sql:
+ "insert into root.t1.wf01.wt01(timestamp, s1) values(1, 1.0), (2, 'hello')"
+ we want the type of 's1' to be inferred as FLOAT(1.0) rather than TEXT('hello').
+ */
Review Comment:
The case is tested in `IoTDBInsertAlignedValuesIT.testInsertMultiRows()` and `IoTDBInsertMultiRowIT.testInsertMultiRowWithMisMatchDataType()`.
--
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
[GitHub] [iotdb] HTHou commented on a diff in pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
HTHou commented on code in PR #7918:
URL: https://github.com/apache/iotdb/pull/7918#discussion_r1018045495
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java:
##########
@@ -1251,16 +1252,18 @@ public Analysis visitInsert(InsertStatement insertStatement, MPPQueryContext con
System.arraycopy(measurementList, 0, measurements, 0, measurements.length);
statement.setMeasurements(measurements);
statement.setTime(timeArray[i]);
- statement.setDataTypes(new TSDataType[measurementList.length]);
+ TSDataType[] dataTypes = new TSDataType[measurementList.length];
+ Arrays.fill(dataTypes, TSDataType.TEXT);
Review Comment:
Do we need to fill these dataTypes? The correct types should be inferred or gotten in SchemaValidate step.
--
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
[GitHub] [iotdb] DanielWang2035 commented on a diff in pull request #7918: [IOTDB-4558] Use insertRowsOfOneDevice to execute insert multi rows sql statement
Posted by GitBox <gi...@apache.org>.
DanielWang2035 commented on code in PR #7918:
URL: https://github.com/apache/iotdb/pull/7918#discussion_r1018551227
##########
server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java:
##########
@@ -1251,16 +1252,18 @@ public Analysis visitInsert(InsertStatement insertStatement, MPPQueryContext con
System.arraycopy(measurementList, 0, measurements, 0, measurements.length);
statement.setMeasurements(measurements);
statement.setTime(timeArray[i]);
- statement.setDataTypes(new TSDataType[measurementList.length]);
+ TSDataType[] dataTypes = new TSDataType[measurementList.length];
+ Arrays.fill(dataTypes, TSDataType.TEXT);
Review Comment:
Yes we need, or there will be runtime exceptions in `storeMeasurementsAndDataType()`. Filling dataTypes here won't make difference to the inferring step.
--
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