[GitHub] [ignite] ptupitsyn commented on a change in pull request #9862: IGNITE-16608 Connection timeout affects statement execution

[GitBox <gi...@apache.org>]

ptupitsyn commented on a change in pull request #9862:
URL: https://github.com/apache/ignite/pull/9862#discussion_r818363871



##########
File path: modules/platforms/cpp/odbc/include/ignite/odbc/connection.h
##########
@@ -204,25 +204,27 @@ namespace ignite
              *
              * @param req Request message.
              * @param rsp Response message.
-             * @param timeout Timeout.
+             * @param timeout Timeout. 0 means disabled and -1 means to use default connection timeout.

Review comment:
       ```suggestion
                * @param timeout0 Timeout. 0 means disabled and -1 means to use default connection timeout.
   ```
   On the other hand, `timeout0` as a parameter name looks weird, maybe we should use different variable names?

##########
File path: modules/platforms/cpp/odbc/src/query/batch_query.cpp
##########
@@ -144,23 +144,12 @@ namespace ignite
             {
                 const std::string& schema = connection.GetSchema();
 
-                QueryExecuteBatchRequest req(schema, sql, params, begin, end, last, timeout,
-                    connection.IsAutoCommit());
+                QueryExecuteBatchRequest req(schema, sql, params, begin, end, last, timeout, connection.IsAutoCommit());
                 QueryExecuteBatchResponse rsp;
 
                 try
                 {
-                    // Setting connection timeout to 1 second more than query timeout itself.
-                    int32_t connectionTimeout = timeout ? timeout + 1 : 0;
-
-                    bool success = connection.SyncMessage(req, rsp, connectionTimeout);
-
-                    if (!success)
-                    {
-                        diag.AddStatusRecord(SqlState::SHYT00_TIMEOUT_EXPIRED, "Query timeout expired");
-
-                        return SqlResult::AI_ERROR;
-                    }
+                    connection.SyncMessage(req, rsp);

Review comment:
       Can you please explain the fix? We no longer handle the timeout here, how does it help?




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org