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/04/25 07:54:47 UTC

[GitHub] [iotdb] haimeiguo opened a new pull request #3059: JDBC bug - check authority for execute batch

haimeiguo opened a new pull request #3059:
URL: https://github.com/apache/iotdb/pull/3059


   1. when user does not have "create timeseries" privilege, JDBC allows user to create timeseries using executeBatch() interface. this adds authority check
   2. when multiple errors occur, JDBC only shows user the last error message. this adds all error message with its SQL. 
   ![image](https://user-images.githubusercontent.com/68632589/115985369-2959b600-a5de-11eb-9b4c-81af85d0f3fd.png)
   


-- 
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



[GitHub] [iotdb] neuyilan commented on a change in pull request #3059: JDBC bug - check authority for execute batch

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3059:
URL: https://github.com/apache/iotdb/pull/3059#discussion_r619919894



##########
File path: jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBStatement.java
##########
@@ -322,14 +322,19 @@ private boolean executeSQL(String sql) throws TException, SQLException {
     TSStatus execResp = client.executeBatchStatement(execReq);
     int[] result = new int[batchSQLList.size()];
     boolean allSuccess = true;
-    String message = "";
+    String message = "\n";
     for (int i = 0; i < result.length; i++) {
       if (execResp.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
         result[i] = execResp.getSubStatus().get(i).code;
         if (result[i] != TSStatusCode.SUCCESS_STATUS.getStatusCode()
             && result[i] != TSStatusCode.NEED_REDIRECTION.getStatusCode()) {
           allSuccess = false;
-          message = execResp.getSubStatus().get(i).message;
+          message +=

Review comment:
       Use StringBuilde insteand of + 

##########
File path: jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBStatement.java
##########
@@ -322,14 +322,19 @@ private boolean executeSQL(String sql) throws TException, SQLException {
     TSStatus execResp = client.executeBatchStatement(execReq);
     int[] result = new int[batchSQLList.size()];
     boolean allSuccess = true;
-    String message = "";
+    String message = "\n";
     for (int i = 0; i < result.length; i++) {
       if (execResp.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
         result[i] = execResp.getSubStatus().get(i).code;
         if (result[i] != TSStatusCode.SUCCESS_STATUS.getStatusCode()
             && result[i] != TSStatusCode.NEED_REDIRECTION.getStatusCode()) {
           allSuccess = false;
-          message = execResp.getSubStatus().get(i).message;
+          message +=

Review comment:
       Use StringBuilder insteand of + 

##########
File path: jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBStatement.java
##########
@@ -322,31 +322,32 @@ private boolean executeSQL(String sql) throws TException, SQLException {
     TSStatus execResp = client.executeBatchStatement(execReq);
     int[] result = new int[batchSQLList.size()];
     boolean allSuccess = true;
-    String message = "\n";
+    StringBuilder message = new StringBuilder("\n");
     for (int i = 0; i < result.length; i++) {
       if (execResp.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
         result[i] = execResp.getSubStatus().get(i).code;
         if (result[i] != TSStatusCode.SUCCESS_STATUS.getStatusCode()
             && result[i] != TSStatusCode.NEED_REDIRECTION.getStatusCode()) {
           allSuccess = false;
-          message +=
+          message.append(
               execResp.getSubStatus().get(i).message
                   + " for SQL: \""
                   + batchSQLList.get(i)
                   + "\""
-                  + "\n";
+                  + "\n");

Review comment:
       message.append(execResp.getSubStatus().get(i).message).append(" for SQL: \"")
                 .append(batchSQLList.get(i)).append("\"").append("\n");




-- 
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



[GitHub] [iotdb] neuyilan commented on a change in pull request #3059: JDBC bug - check authority for execute batch

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3059:
URL: https://github.com/apache/iotdb/pull/3059#discussion_r619919894



##########
File path: jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBStatement.java
##########
@@ -322,14 +322,19 @@ private boolean executeSQL(String sql) throws TException, SQLException {
     TSStatus execResp = client.executeBatchStatement(execReq);
     int[] result = new int[batchSQLList.size()];
     boolean allSuccess = true;
-    String message = "";
+    String message = "\n";
     for (int i = 0; i < result.length; i++) {
       if (execResp.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
         result[i] = execResp.getSubStatus().get(i).code;
         if (result[i] != TSStatusCode.SUCCESS_STATUS.getStatusCode()
             && result[i] != TSStatusCode.NEED_REDIRECTION.getStatusCode()) {
           allSuccess = false;
-          message = execResp.getSubStatus().get(i).message;
+          message +=

Review comment:
       Use StringBuilde insteand of + 




-- 
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



[GitHub] [iotdb] mychaow merged pull request #3059: JDBC bug - check authority for execute batch

Posted by GitBox <gi...@apache.org>.
mychaow merged pull request #3059:
URL: https://github.com/apache/iotdb/pull/3059


   


-- 
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



[GitHub] [iotdb] neuyilan commented on a change in pull request #3059: JDBC bug - check authority for execute batch

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3059:
URL: https://github.com/apache/iotdb/pull/3059#discussion_r619919894



##########
File path: jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBStatement.java
##########
@@ -322,14 +322,19 @@ private boolean executeSQL(String sql) throws TException, SQLException {
     TSStatus execResp = client.executeBatchStatement(execReq);
     int[] result = new int[batchSQLList.size()];
     boolean allSuccess = true;
-    String message = "";
+    String message = "\n";
     for (int i = 0; i < result.length; i++) {
       if (execResp.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
         result[i] = execResp.getSubStatus().get(i).code;
         if (result[i] != TSStatusCode.SUCCESS_STATUS.getStatusCode()
             && result[i] != TSStatusCode.NEED_REDIRECTION.getStatusCode()) {
           allSuccess = false;
-          message = execResp.getSubStatus().get(i).message;
+          message +=

Review comment:
       Use StringBuilder insteand of + 




-- 
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