Merge pull request #55558 from ClickHouse/vdimir/mssql_odbc_cursor

Fix 'Invalid cursor state' in odbc interacting with MS SQL Server
ClickHouse · Oct 17, 2023 · 1d46ed7 · 1d46ed7
2 parents b5693d3 + aae3894
commit 1d46ed7
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 6 deletions.
diff --git a/programs/odbc-bridge/ColumnInfoHandler.cpp b/programs/odbc-bridge/ColumnInfoHandler.cpp
@@ -145,6 +145,10 @@ void ODBCColumnsInfoHandler::handleRequest(HTTPServerRequest & request, HTTPServ
             if (tables.next())
             {
                 catalog_name = tables.table_catalog();
+                /// `tables.next()` call is mandatory to drain the iterator before next operation and avoid "Invalid cursor state"
+                if (tables.next())
+                    throw Exception(ErrorCodes::UNKNOWN_TABLE, "Driver returned more than one table for '{}': '{}' and '{}'",
+                                    table_name, catalog_name, tables.table_schema());
                 LOG_TRACE(log, "Will fetch info for table '{}.{}'", catalog_name, table_name);
                 return catalog.find_columns(/* column = */ "", table_name, /* schema = */ "", catalog_name);
             }
@@ -153,6 +157,10 @@ void ODBCColumnsInfoHandler::handleRequest(HTTPServerRequest & request, HTTPServ
             if (tables.next())
             {
                 catalog_name = tables.table_catalog();
+                /// `tables.next()` call is mandatory to drain the iterator before next operation and avoid "Invalid cursor state"
+                if (tables.next())
+                    throw Exception(ErrorCodes::UNKNOWN_TABLE, "Driver returned more than one table for '{}': '{}' and '{}'",
+                                    table_name, catalog_name, tables.table_schema());
                 LOG_TRACE(log, "Will fetch info for table '{}.{}.{}'", catalog_name, schema_name, table_name);
                 return catalog.find_columns(/* column = */ "", table_name, schema_name, catalog_name);
             }

diff --git a/programs/odbc-bridge/ODBCPooledConnectionFactory.h b/programs/odbc-bridge/ODBCPooledConnectionFactory.h
@@ -91,16 +91,17 @@ T execute(nanodbc::ConnectionHolderPtr connection_holder, std::function<T(nanodb
     }
     catch (const nanodbc::database_error & e)
     {
-        LOG_ERROR(
-            &Poco::Logger::get("ODBCConnection"),
-            "ODBC query failed with error: {}, state: {}, native code: {}",
-            e.what(), e.state(), e.native());
-
         /// SQLState, connection related errors start with 08 (main: 08S01), cursor invalid state is 24000.
         /// Invalid cursor state is a retriable error.
         /// Invalid transaction state 25000. Truncate to 2 letters on purpose.
         /// https://docs.microsoft.com/ru-ru/sql/odbc/reference/appendixes/appendix-a-odbc-error-codes?view=sql-server-ver15
-        if (e.state().starts_with("08") || e.state().starts_with("24") || e.state().starts_with("25"))
+        bool is_retriable = e.state().starts_with("08") || e.state().starts_with("24") || e.state().starts_with("25");
+        LOG_ERROR(
+            &Poco::Logger::get("ODBCConnection"),
+            "ODBC query failed with error: {}, state: {}, native code: {}{}",
+            e.what(), e.state(), e.native(), is_retriable ? ", will retry" : "");
+
+        if (is_retriable)
         {
             connection_holder->updateConnection();
             return query_func(connection_holder->get());