GH-47707: [C++][FlightRPC] Add tests for descriptor handle allocation (#48053)

### Rationale for this change
Add tests for  `SQL_HANDLE_DESC` descriptor handle allocation, and diagnostics test. Related test updates

### What changes are included in this PR?
- tests for  `SQL_HANDLE_DESC` 
- Use `nullptr` explicitly where applicable
- We use error associated returned from Driver manager to ensure users can get errors from descriptor handle
### Are these changes tested?
Tested on local MSVC Windows
### Are there any user-facing changes?
N/A

* GitHub Issue: #47707

Authored-by: Alina (Xi) Li <alina.li@improving.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

Author: alinaliBQ

cpp/src/arrow/flight/sql/odbc/odbc_api.cc

@@ -44,9 +44,6 @@ SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE* result)
   // GH-47706 TODO: Add tests for SQLAllocStmt, pre-requisite requires
   // SQLDriverConnect implementation
 
-  // GH-47707 TODO: Add tests for SQL_HANDLE_DESC implementation for
-  // descriptor handle, pre-requisite requires SQLAllocStmt
-
   *result = nullptr;
 
   switch (type) {
@@ -147,8 +144,6 @@ SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle) {
   // GH-47706 TODO: Add tests for SQLFreeStmt, pre-requisite requires
   // SQLAllocStmt tests
 
-  // GH-47707 TODO: Add tests for SQL_HANDLE_DESC implementation for
-  // descriptor handle
   switch (type) {
     case SQL_HANDLE_ENV: {
       using ODBC::ODBCEnvironment;

cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.h

@@ -67,6 +67,9 @@ class ODBCStatement : public ODBCHandle<ODBCStatement> {
   void SetStmtAttr(SQLINTEGER statement_attribute, SQLPOINTER value,
                    SQLINTEGER buffer_size, bool is_unicode);
 
+  /// \brief Revert back to implicitly allocated internal descriptors.
+  /// isApd as True indicates APD descritor is to be reverted.
+  /// isApd as False indicates ARD descritor is to be reverted.
   void RevertAppDescriptor(bool is_apd);
 
   inline ODBCDescriptor* GetIRD() { return ird_.get(); }

cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc

@@ -146,23 +146,25 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrTxnIsolationUnsupported
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrDbcInfoTokenSetOnly) {
   // Verify that set-only attribute cannot be read
   SQLPOINTER ptr = NULL;
-  ASSERT_EQ(SQL_ERROR, SQLGetConnectAttr(this->conn, SQL_ATTR_DBC_INFO_TOKEN, ptr, 0, 0));
+  ASSERT_EQ(SQL_ERROR,
+            SQLGetConnectAttr(this->conn, SQL_ATTR_DBC_INFO_TOKEN, ptr, 0, nullptr));
   VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHY092);
 }
 #endif
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrOdbcCursorsDMOnly) {
   // Verify that DM-only attribute is handled by driver manager
   SQLULEN cursor_attr;
-  ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ODBC_CURSORS, &cursor_attr, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_ODBC_CURSORS,
+                                           &cursor_attr, 0, nullptr));
   EXPECT_EQ(SQL_CUR_USE_DRIVER, cursor_attr);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTraceDMOnly) {
   // Verify that DM-only attribute is handled by driver manager
   SQLUINTEGER trace;
-  ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_TRACE, &trace, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS,
+            SQLGetConnectAttr(this->conn, SQL_ATTR_TRACE, &trace, 0, nullptr));
   EXPECT_EQ(SQL_OPT_TRACE_OFF, trace);
 }
 
@@ -190,15 +192,15 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTranslateLibUnsupported
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTranslateOptionUnsupported) {
   SQLINTEGER option;
-  ASSERT_EQ(SQL_ERROR,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_TRANSLATE_OPTION, &option, 0, 0));
+  ASSERT_EQ(SQL_ERROR, SQLGetConnectAttr(this->conn, SQL_ATTR_TRANSLATE_OPTION, &option,
+                                         0, nullptr));
   VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrTxnIsolationUnsupported) {
   SQLINTEGER isolation;
-  ASSERT_EQ(SQL_ERROR,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_TXN_ISOLATION, &isolation, 0, 0));
+  ASSERT_EQ(SQL_ERROR, SQLGetConnectAttr(this->conn, SQL_ATTR_TXN_ISOLATION, &isolation,
+                                         0, nullptr));
   VerifyOdbcErrorState(SQL_HANDLE_DBC, this->conn, kErrorStateHYC00);
 }
 
@@ -219,7 +221,7 @@ TYPED_TEST(ConnectionAttributeTest,
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAsyncDbcEventDefault) {
   SQLPOINTER ptr = NULL;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_EVENT, ptr, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_EVENT, ptr, 0, nullptr));
   EXPECT_EQ(reinterpret_cast<SQLPOINTER>(NULL), ptr);
 }
 #endif
@@ -228,7 +230,7 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAsyncDbcEventDefault) {
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAsyncDbcPcallbackDefault) {
   SQLPOINTER ptr = NULL;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCALLBACK, ptr, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCALLBACK, ptr, 0, nullptr));
   EXPECT_EQ(reinterpret_cast<SQLPOINTER>(NULL), ptr);
 }
 #endif
@@ -237,41 +239,43 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAsyncDbcPcallbackDefaul
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAsyncDbcPcontextDefault) {
   SQLPOINTER ptr = NULL;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCONTEXT, ptr, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_DBC_PCONTEXT, ptr, 0, nullptr));
   EXPECT_EQ(reinterpret_cast<SQLPOINTER>(NULL), ptr);
 }
 #endif
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAsyncEnableDefault) {
   SQLULEN enable;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_ENABLE, &enable, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ASYNC_ENABLE, &enable, 0, nullptr));
   EXPECT_EQ(SQL_ASYNC_ENABLE_OFF, enable);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAutoIpdDefault) {
   SQLUINTEGER ipd;
-  ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_AUTO_IPD, &ipd, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS,
+            SQLGetConnectAttr(this->conn, SQL_ATTR_AUTO_IPD, &ipd, 0, nullptr));
   EXPECT_EQ(static_cast<SQLUINTEGER>(SQL_FALSE), ipd);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrAutocommitDefault) {
   SQLUINTEGER auto_commit;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_AUTOCOMMIT, &auto_commit, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_AUTOCOMMIT, &auto_commit, 0, nullptr));
   EXPECT_EQ(SQL_AUTOCOMMIT_ON, auto_commit);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrEnlistInDtcDefault) {
   SQLPOINTER ptr = NULL;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ENLIST_IN_DTC, ptr, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ENLIST_IN_DTC, ptr, 0, nullptr));
   EXPECT_EQ(reinterpret_cast<SQLPOINTER>(NULL), ptr);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLGetConnectAttrQuietModeDefault) {
   HWND ptr = NULL;
-  ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_QUIET_MODE, ptr, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS,
+            SQLGetConnectAttr(this->conn, SQL_ATTR_QUIET_MODE, ptr, 0, nullptr));
   EXPECT_EQ(reinterpret_cast<SQLPOINTER>(NULL), ptr);
 }
 
@@ -281,7 +285,7 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAccessModeValid) {
   // Check default value first
   SQLUINTEGER mode = -1;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ACCESS_MODE, &mode, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ACCESS_MODE, &mode, 0, nullptr));
   EXPECT_EQ(SQL_MODE_READ_WRITE, mode);
 
   ASSERT_EQ(SQL_SUCCESS,
@@ -290,7 +294,7 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAccessModeValid) {
 
   mode = -1;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_ACCESS_MODE, &mode, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_ACCESS_MODE, &mode, 0, nullptr));
   EXPECT_EQ(SQL_MODE_READ_WRITE, mode);
 
   // Attempt to set to SQL_MODE_READ_ONLY, driver should return warning and not error
@@ -305,32 +309,32 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrAccessModeValid) {
 TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrConnectionTimeoutValid) {
   // Check default value first
   SQLUINTEGER timeout = -1;
-  ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_CONNECTION_TIMEOUT, &timeout, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_CONNECTION_TIMEOUT,
+                                           &timeout, 0, nullptr));
   EXPECT_EQ(0, timeout);
 
   ASSERT_EQ(SQL_SUCCESS, SQLSetConnectAttr(this->conn, SQL_ATTR_CONNECTION_TIMEOUT,
                                            reinterpret_cast<SQLPOINTER>(42), 0));
 
   timeout = -1;
-  ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_CONNECTION_TIMEOUT, &timeout, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS, SQLGetConnectAttr(this->conn, SQL_ATTR_CONNECTION_TIMEOUT,
+                                           &timeout, 0, nullptr));
   EXPECT_EQ(42, timeout);
 }
 
 TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrLoginTimeoutValid) {
   // Check default value first
   SQLUINTEGER timeout = -1;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_LOGIN_TIMEOUT, &timeout, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_LOGIN_TIMEOUT, &timeout, 0, nullptr));
   EXPECT_EQ(0, timeout);
 
   ASSERT_EQ(SQL_SUCCESS, SQLSetConnectAttr(this->conn, SQL_ATTR_LOGIN_TIMEOUT,
                                            reinterpret_cast<SQLPOINTER>(42), 0));
 
   timeout = -1;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_LOGIN_TIMEOUT, &timeout, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_LOGIN_TIMEOUT, &timeout, 0, nullptr));
   EXPECT_EQ(42, timeout);
 }
 
@@ -340,15 +344,15 @@ TYPED_TEST(ConnectionAttributeTest, TestSQLSetConnectAttrPacketSizeValid) {
   // Check default value first
   SQLUINTEGER size = -1;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_PACKET_SIZE, &size, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_PACKET_SIZE, &size, 0, nullptr));
   EXPECT_EQ(0, size);
 
   ASSERT_EQ(SQL_SUCCESS, SQLSetConnectAttr(this->conn, SQL_ATTR_PACKET_SIZE,
                                            reinterpret_cast<SQLPOINTER>(0), 0));
 
   size = -1;
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetConnectAttr(this->conn, SQL_ATTR_PACKET_SIZE, &size, 0, 0));
+            SQLGetConnectAttr(this->conn, SQL_ATTR_PACKET_SIZE, &size, 0, nullptr));
   EXPECT_EQ(0, size);
 
   // Attempt to set to non-zero value, driver should return warning and not error

cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc

@@ -166,7 +166,8 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDriverHdesc) {
 
   // Value returned from driver manager is the desc address
   SQLHDESC local_desc = descriptor;
-  ASSERT_EQ(SQL_SUCCESS, SQLGetInfo(this->conn, SQL_HANDLE_DESC, &local_desc, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS,
+            SQLGetInfo(this->conn, SQL_HANDLE_DESC, &local_desc, 0, nullptr));
   EXPECT_GT(local_desc, static_cast<SQLHSTMT>(0));
 
   // Free descriptor handle
@@ -194,7 +195,8 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDriverHlib) {
 TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDriverHstmt) {
   // Value returned from driver manager is the stmt address
   SQLHSTMT local_stmt = this->stmt;
-  ASSERT_EQ(SQL_SUCCESS, SQLGetInfo(this->conn, SQL_DRIVER_HSTMT, &local_stmt, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS,
+            SQLGetInfo(this->conn, SQL_DRIVER_HSTMT, &local_stmt, 0, nullptr));
   EXPECT_GT(local_stmt, static_cast<SQLHSTMT>(0));
 }
 

cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc

@@ -117,7 +117,7 @@ TEST(SQLGetEnvAttr, TestSQLGetEnvAttrODBCVersion) {
   // Allocate an environment handle
   ASSERT_EQ(SQL_SUCCESS, SQLAllocEnv(&env));
 
-  ASSERT_EQ(SQL_SUCCESS, SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, &version, 0, 0));
+  ASSERT_EQ(SQL_SUCCESS, SQLGetEnvAttr(env, SQL_ATTR_ODBC_VERSION, &version, 0, nullptr));
 
   ASSERT_EQ(SQL_OV_ODBC2, version);
 
@@ -155,7 +155,7 @@ TYPED_TEST(ConnectionTest, TestSQLGetEnvAttrOutputNTS) {
   SQLINTEGER output_nts;
 
   ASSERT_EQ(SQL_SUCCESS,
-            SQLGetEnvAttr(this->env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, 0));
+            SQLGetEnvAttr(this->env, SQL_ATTR_OUTPUT_NTS, &output_nts, 0, nullptr));
 
   ASSERT_EQ(SQL_TRUE, output_nts);
 }
@@ -432,7 +432,7 @@ TEST_F(ConnectionRemoteTest, TestSQLConnectDSNPrecedence) {
       << GetOdbcErrorMessage(SQL_HANDLE_DBC, conn);
 }
 
-#endif
+#endif  // _WIN32
 
 TEST_F(ConnectionRemoteTest, TestSQLDriverConnectInvalidUid) {
   // Invalid connect string
@@ -508,4 +508,71 @@ TYPED_TEST(ConnectionHandleTest, TestCloseConnectionWithOpenStatement) {
   ASSERT_EQ(SQL_SUCCESS, SQLDisconnect(this->conn));
 }
 
+TYPED_TEST(ConnectionTest, TestSQLAllocFreeDesc) {
+  SQLHDESC descriptor;
+
+  // Allocate a descriptor using alloc handle
+  ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_DESC, this->conn, &descriptor));
+
+  // Free descriptor handle
+  ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DESC, descriptor));
+}
+
+TYPED_TEST(ConnectionTest, TestSQLSetStmtAttrDescriptor) {
+  SQLHDESC apd_descriptor, ard_descriptor;
+
+  // Allocate an APD descriptor using alloc handle
+  ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_DESC, this->conn, &apd_descriptor));
+
+  // Allocate an ARD descriptor using alloc handle
+  ASSERT_EQ(SQL_SUCCESS, SQLAllocHandle(SQL_HANDLE_DESC, this->conn, &ard_descriptor));
+
+  // Save implicitly allocated internal APD and ARD descriptor pointers
+  SQLPOINTER internal_apd, internal_ard = nullptr;
+
+  EXPECT_EQ(SQL_SUCCESS, SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_PARAM_DESC,
+                                        &internal_apd, sizeof(internal_apd), 0));
+
+  EXPECT_EQ(SQL_SUCCESS, SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_ROW_DESC, &internal_ard,
+                                        sizeof(internal_ard), 0));
+
+  // Set APD descriptor to explicitly allocated handle
+  EXPECT_EQ(SQL_SUCCESS, SQLSetStmtAttr(this->stmt, SQL_ATTR_APP_PARAM_DESC,
+                                        reinterpret_cast<SQLPOINTER>(apd_descriptor), 0));
+
+  // Set ARD descriptor to explicitly allocated handle
+  EXPECT_EQ(SQL_SUCCESS, SQLSetStmtAttr(this->stmt, SQL_ATTR_APP_ROW_DESC,
+                                        reinterpret_cast<SQLPOINTER>(ard_descriptor), 0));
+
+  // Verify APD and ARD descriptors are set to explicitly allocated pointers
+  SQLPOINTER value = nullptr;
+  EXPECT_EQ(SQL_SUCCESS, SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_PARAM_DESC, &value,
+                                        sizeof(value), 0));
+
+  EXPECT_EQ(apd_descriptor, value);
+
+  EXPECT_EQ(SQL_SUCCESS,
+            SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_ROW_DESC, &value, sizeof(value), 0));
+
+  EXPECT_EQ(ard_descriptor, value);
+
+  // Free explicitly allocated APD and ARD descriptor handles
+  ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DESC, apd_descriptor));
+
+  ASSERT_EQ(SQL_SUCCESS, SQLFreeHandle(SQL_HANDLE_DESC, ard_descriptor));
+
+  // Verify APD and ARD descriptors has been reverted to implicit descriptors
+  value = nullptr;
+
+  EXPECT_EQ(SQL_SUCCESS, SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_PARAM_DESC, &value,
+                                        sizeof(value), 0));
+
+  EXPECT_EQ(internal_apd, value);
+
+  EXPECT_EQ(SQL_SUCCESS,
+            SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_ROW_DESC, &value, sizeof(value), 0));
+
+  EXPECT_EQ(internal_ard, value);
+}
+
 }  // namespace arrow::flight::sql::odbc