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 2021/02/23 12:59:36 UTC
[GitHub] [iotdb] neuyilan commented on a change in pull request #2725: [IOTDB-1164]Optimize the executeBatch interface in JDBC
neuyilan commented on a change in pull request #2725:
URL: https://github.com/apache/iotdb/pull/2725#discussion_r581014204
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -441,12 +441,31 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) {
List<TSStatus> result = new ArrayList<>();
boolean isAllSuccessful = true;
+ InsertRowsPlan insertRowsPlan = new InsertRowsPlan();
+ int index = 0;
for (String statement : req.getStatements()) {
- long t2 = System.currentTimeMillis();
- isAllSuccessful =
- executeStatementInBatch(statement, result, req.getSessionId()) && isAllSuccessful;
- Measurement.INSTANCE.addOperationLatency(Operation.EXECUTE_ONE_SQL_IN_BATCH, t2);
+ PhysicalPlan physicalPlan =
+ processor.parseSQLToPhysicalPlan(
+ statement, sessionIdZoneIdMap.get(req.getSessionId()), DEFAULT_FETCH_SIZE);
+ if (physicalPlan.getOperatorType().equals(INSERT)) {
+ insertRowsPlan.addOneInsertRowPlan((InsertRowPlan) physicalPlan, index);
+ index++;
+ } else {
+ long t2 = System.currentTimeMillis();
+ isAllSuccessful =
+ executeStatementInBatch(statement, result, req.getSessionId()) && isAllSuccessful;
Review comment:
We have parsed the statement to the plan, so we can pass the plan to the `executeStatementInBatch()` method instead of recalled the `parseSQLToPhysicalPlan()` method in executeStatementInBatch().
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -441,12 +441,31 @@ public TSStatus executeBatchStatement(TSExecuteBatchStatementReq req) {
List<TSStatus> result = new ArrayList<>();
boolean isAllSuccessful = true;
+ InsertRowsPlan insertRowsPlan = new InsertRowsPlan();
+ int index = 0;
for (String statement : req.getStatements()) {
- long t2 = System.currentTimeMillis();
- isAllSuccessful =
- executeStatementInBatch(statement, result, req.getSessionId()) && isAllSuccessful;
- Measurement.INSTANCE.addOperationLatency(Operation.EXECUTE_ONE_SQL_IN_BATCH, t2);
+ PhysicalPlan physicalPlan =
+ processor.parseSQLToPhysicalPlan(
+ statement, sessionIdZoneIdMap.get(req.getSessionId()), DEFAULT_FETCH_SIZE);
+ if (physicalPlan.getOperatorType().equals(INSERT)) {
+ insertRowsPlan.addOneInsertRowPlan((InsertRowPlan) physicalPlan, index);
+ index++;
+ } else {
+ long t2 = System.currentTimeMillis();
+ isAllSuccessful =
+ executeStatementInBatch(statement, result, req.getSessionId()) && isAllSuccessful;
+ Measurement.INSTANCE.addOperationLatency(Operation.EXECUTE_ONE_SQL_IN_BATCH, t2);
+ }
Review comment:
When we encounter a non InsertPlan, we should first submit the InsertRowsPlan above, and then execute this statement. For example, the following statement:
```
insert into root.ln.wf02 .wt02(timestamp,status) values(1,true);
insert into root.ln.wf02 .wt02(timestamp,status) values(2,true);
delete timeseries root.ln.wf03 .wt03;
insert into root.ln.wf03 .wt03(timestamp,status) values(3,true);
```
We should submit the above two insertions first request, and then execute the delete root.ln.wf03.wt03 request, and finally execute the insert root.ln.wf03.wt03 request. Instead of performing the delete operation first and the insert operation first, the semantics will be different.
----------------------------------------------------------------
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