From d9d78bc5f46a005c7e5a69c2185262deee2d8355 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 4 Jul 2025 14:46:39 -0700 Subject: [PATCH 1/9] SQLBindCol Initial Impl --- cpp/src/arrow/flight/sql/odbc/entry_points.cc | 14 +++++--------- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 10 ++++++++++ cpp/src/arrow/flight/sql/odbc/odbc_api.h | 2 ++ .../arrow/flight/sql/odbc/tests/statement_test.cc | 5 +++++ 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/entry_points.cc b/cpp/src/arrow/flight/sql/odbc/entry_points.cc index e7c55d5a73a1..ec30405870f6 100644 --- a/cpp/src/arrow/flight/sql/odbc/entry_points.cc +++ b/cpp/src/arrow/flight/sql/odbc/entry_points.cc @@ -150,15 +150,11 @@ SQLRETURN SQL_API SQLPrepare(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER text SQLRETURN SQL_API SQLExecute(SQLHSTMT stmt) { return arrow::SQLExecute(stmt); } -SQLRETURN SQL_API SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT columnNumber, - SQLSMALLINT targetType, SQLPOINTER targetValuePtr, - SQLLEN bufferLength, SQLLEN* strLen_or_IndPtr) { - LOG_DEBUG( - "SQLBindCol called with stmt: {}, columnNumber: {}, targetType: {}, " - "targetValuePtr: {}, bufferLength: {}, strLen_or_IndPtr: {}", - stmt, columnNumber, targetType, targetValuePtr, bufferLength, - fmt::ptr(strLen_or_IndPtr)); - return SQL_ERROR; +SQLRETURN SQL_API SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType, + SQLPOINTER dataPtr, SQLLEN bufferLength, + SQLLEN* indicatorPtr) { + return arrow::SQLBindCol(stmt, recordNumber, cType, dataPtr, bufferLength, + indicatorPtr); } SQLRETURN SQL_API SQLCancel(SQLHSTMT stmt) { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 91121726e8f1..de033a5bfbf3 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -976,6 +976,16 @@ SQLRETURN SQLFetch(SQLHSTMT stmt) { }); } +SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType, + SQLPOINTER dataPtr, SQLLEN bufferLength, SQLLEN* indicatorPtr) { + LOG_DEBUG( + "SQLBindCol called with stmt: {}, recordNumber: {}, cType: {}, " + "dataPtr: {}, bufferLength: {}, strLen_or_IndPtr: {}", + stmt, recordNumber, cType, dataPtr, bufferLength, fmt::ptr(indicatorPtr)); + return SQL_ERROR; + //-AL - TODO: implement SQLBindCol functionality +} + SQLRETURN SQLGetData(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType, SQLPOINTER dataPtr, SQLLEN bufferLength, SQLLEN* indicatorPtr) { // GH-46979: support SQL_C_GUID data type diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.h b/cpp/src/arrow/flight/sql/odbc/odbc_api.h index 504a8f545f88..91c45b887389 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.h +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.h @@ -68,6 +68,8 @@ SQLRETURN SQLExecDirect(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER textLengt SQLRETURN SQLPrepare(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER textLength); SQLRETURN SQLExecute(SQLHSTMT stmt); SQLRETURN SQLFetch(SQLHSTMT stmt); +SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType, + SQLPOINTER dataPtr, SQLLEN bufferLength, SQLLEN* indicatorPtr); SQLRETURN SQLGetData(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType, SQLPOINTER dataPtr, SQLLEN bufferLength, SQLLEN* indicatorPtr); SQLRETURN SQLMoreResults(SQLHSTMT stmt); diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 3335d5f72469..74f131baa4e0 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1432,4 +1432,9 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLExecDirectIgnoreInvalidBufLen) { this->disconnect(); } +//-AL- TODO: add tests that use SQLBindCol to fetch all data . +// TODO: add tests that replicate queries for 1) varibinary (mock server) +// +// and 2) time (remote server) + } // namespace arrow::flight::sql::odbc From dd844717920e73840414d92932e4c58f0d0f651c Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 4 Jul 2025 15:47:52 -0700 Subject: [PATCH 2/9] Draft SQLBindCol impl + tests --- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 10 +- .../flight/sql/odbc/tests/statement_test.cc | 318 +++++++++++++++++- 2 files changed, 325 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index de033a5bfbf3..d748c0f99c3f 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -982,8 +982,14 @@ SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType "SQLBindCol called with stmt: {}, recordNumber: {}, cType: {}, " "dataPtr: {}, bufferLength: {}, strLen_or_IndPtr: {}", stmt, recordNumber, cType, dataPtr, bufferLength, fmt::ptr(indicatorPtr)); - return SQL_ERROR; - //-AL - TODO: implement SQLBindCol functionality + using ODBC::ODBCDescriptor; + using ODBC::ODBCStatement; + return ODBCStatement::ExecuteWithDiagnostics(stmt, SQL_ERROR, [=]() { + ODBCStatement* statement = reinterpret_cast(stmt); + ODBCDescriptor* ard = statement->GetARD(); + ard->BindCol(recordNumber, cType, dataPtr, bufferLength, indicatorPtr); + return SQL_SUCCESS; + }); } SQLRETURN SQLGetData(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType, diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 74f131baa4e0..6e35cb1925d2 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1432,9 +1432,325 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLExecDirectIgnoreInvalidBufLen) { this->disconnect(); } -//-AL- TODO: add tests that use SQLBindCol to fetch all data . +//-AL- TODOs for tests // TODO: add tests that replicate queries for 1) varibinary (mock server) // // and 2) time (remote server) +// TODO: add tests for indicator pointer for null data etc +// TODO: add SQLBindCol tests for row fetching + +TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColSimpleQuery) { + // -AL- TODO can perhaps delete this test later after TestSQLBindColDataQuery is done, + // this is just proof of concept + this->connect(); + + // Numeric Types + + // Signed Tiny Int + int8_t stiny_int_val; + SQLLEN buf_len = sizeof(stiny_int_val); + SQLLEN ind; + + SQLRETURN ret = + SQLBindCol(this->stmt, 1, SQL_C_STINYINT, &stiny_int_val, buf_len, &ind); + + std::wstring wsql = L"SELECT 1;"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = + SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Check that 1 is fetched + EXPECT_EQ(stiny_int_val, 1); +} + +TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColDataQuery) { + this->connect(); + + // Numeric Types + + // Signed Tiny Int + int8_t stiny_int_val_min; + int8_t stiny_int_val_max; + SQLLEN buf_len = 0; + SQLLEN ind; + + SQLRETURN ret = + SQLBindCol(this->stmt, 1, SQL_C_STINYINT, &stiny_int_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 2, SQL_C_STINYINT, &stiny_int_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Unsigned Tiny Int + uint8_t utiny_int_val_min; + uint8_t utiny_int_val_max; + + ret = SQLBindCol(this->stmt, 3, SQL_C_UTINYINT, &utiny_int_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 4, SQL_C_UTINYINT, &utiny_int_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Signed Small Int + int16_t ssmall_int_val_min; + int16_t ssmall_int_val_max; + + ret = SQLBindCol(this->stmt, 5, SQL_C_SSHORT, &ssmall_int_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 6, SQL_C_SSHORT, &ssmall_int_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Unsigned Small Int + uint16_t usmall_int_val_min; + uint16_t usmall_int_val_max; + + ret = SQLBindCol(this->stmt, 7, SQL_C_USHORT, &usmall_int_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 8, SQL_C_USHORT, &usmall_int_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Signed Integer + SQLINTEGER slong_val_min; + SQLINTEGER slong_val_max; + + ret = SQLBindCol(this->stmt, 9, SQL_C_SLONG, &slong_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 10, SQL_C_SLONG, &slong_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Unsigned Integer + SQLUINTEGER ulong_val_min; + SQLUINTEGER ulong_val_max; + + ret = SQLBindCol(this->stmt, 11, SQL_C_ULONG, &ulong_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 12, SQL_C_ULONG, &ulong_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Signed Big Int + SQLBIGINT sbig_int_val_min; + SQLBIGINT sbig_int_val_max; + + ret = SQLBindCol(this->stmt, 13, SQL_C_SBIGINT, &sbig_int_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 14, SQL_C_SBIGINT, &sbig_int_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Unsigned Big Int + SQLUBIGINT ubig_int_val_min; + SQLUBIGINT ubig_int_val_max; + + ret = SQLBindCol(this->stmt, 15, SQL_C_UBIGINT, &ubig_int_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 16, SQL_C_UBIGINT, &ubig_int_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Decimal + SQL_NUMERIC_STRUCT decimal_val_neg; + SQL_NUMERIC_STRUCT decimal_val_pos; + memset(&decimal_val_neg, 0, sizeof(decimal_val_neg)); + memset(&decimal_val_pos, 0, sizeof(decimal_val_pos)); + + ret = SQLBindCol(this->stmt, 17, SQL_C_NUMERIC, &decimal_val_neg, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 18, SQL_C_NUMERIC, &decimal_val_pos, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Float + float float_val_min; + float float_val_max; + + ret = SQLBindCol(this->stmt, 19, SQL_C_FLOAT, &float_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 20, SQL_C_FLOAT, &float_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Double + SQLDOUBLE double_val_min; + SQLDOUBLE double_val_max; + + ret = SQLBindCol(this->stmt, 21, SQL_C_DOUBLE, &double_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 22, SQL_C_DOUBLE, &double_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Bit + bool bit_val_false; + bool bit_val_true; + + ret = SQLBindCol(this->stmt, 23, SQL_C_BIT, &bit_val_false, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 24, SQL_C_BIT, &bit_val_true, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Characters + SQLCHAR char_val[2]; + buf_len = sizeof(SQLCHAR) * 2; + + ret = SQLBindCol(this->stmt, 25, SQL_C_CHAR, &char_val, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + SQLWCHAR wchar_val[2]; + constexpr size_t wchar_size = driver::odbcabstraction::GetSqlWCharSize(); + buf_len = wchar_size * 2; + + ret = SQLBindCol(this->stmt, 26, SQL_C_WCHAR, &wchar_val, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + SQLWCHAR wvarchar_val[3]; + buf_len = wchar_size * 3; + + ret = SQLBindCol(this->stmt, 27, SQL_C_WCHAR, &wvarchar_val, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + SQLCHAR varchar_val[4]; + buf_len = sizeof(SQLCHAR) * 4; + + ret = SQLBindCol(this->stmt, 28, SQL_C_CHAR, &varchar_val, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Date and Timestamp + SQL_DATE_STRUCT date_val_min{}, date_val_max{}; + buf_len = 0; + + ret = SQLBindCol(this->stmt, 29, SQL_C_TYPE_DATE, &date_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 30, SQL_C_TYPE_DATE, &date_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + SQL_TIMESTAMP_STRUCT timestamp_val_min{}, timestamp_val_max{}; + + ret = + SQLBindCol(this->stmt, 31, SQL_C_TYPE_TIMESTAMP, ×tamp_val_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = + SQLBindCol(this->stmt, 32, SQL_C_TYPE_TIMESTAMP, ×tamp_val_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Execute query and fetch data once since there is only 1 row. + std::wstring wsql = this->getQueryAllDataTypes(); + std::vector sql0(wsql.begin(), wsql.end()); + + ret = + SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Data verification + + // Signed Tiny Int + EXPECT_EQ(stiny_int_val_min, std::numeric_limits::min()); + EXPECT_EQ(stiny_int_val_max, std::numeric_limits::max()); + + // Unsigned Tiny Int + EXPECT_EQ(utiny_int_val_min, std::numeric_limits::min()); + EXPECT_EQ(utiny_int_val_max, std::numeric_limits::max()); + + // Signed Small Int + EXPECT_EQ(ssmall_int_val_min, std::numeric_limits::min()); + EXPECT_EQ(ssmall_int_val_max, std::numeric_limits::max()); + + // Unsigned Small Int + EXPECT_EQ(usmall_int_val_min, std::numeric_limits::min()); + EXPECT_EQ(usmall_int_val_max, std::numeric_limits::max()); + + // Signed Long + EXPECT_EQ(slong_val_min, std::numeric_limits::min()); + EXPECT_EQ(slong_val_max, std::numeric_limits::max()); + + // Unsigned Long + EXPECT_EQ(ulong_val_min, std::numeric_limits::min()); + EXPECT_EQ(ulong_val_max, std::numeric_limits::max()); + + // Signed Big Int + EXPECT_EQ(sbig_int_val_min, std::numeric_limits::min()); + EXPECT_EQ(sbig_int_val_max, std::numeric_limits::max()); + + // Unsigned Big Int + EXPECT_EQ(ubig_int_val_min, std::numeric_limits::min()); + EXPECT_EQ(ubig_int_val_max, std::numeric_limits::max()); + + // Decimal + EXPECT_EQ(decimal_val_neg.sign, 0); + EXPECT_EQ(decimal_val_neg.scale, 0); + EXPECT_EQ(decimal_val_neg.precision, 38); + EXPECT_THAT(decimal_val_neg.val, ::testing::ElementsAre(0xFF, 0xC9, 0x9A, 0x3B, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0)); + + EXPECT_EQ(decimal_val_pos.sign, 1); + EXPECT_EQ(decimal_val_pos.scale, 0); + EXPECT_EQ(decimal_val_pos.precision, 38); + EXPECT_THAT(decimal_val_pos.val, ::testing::ElementsAre(0xFF, 0xC9, 0x9A, 0x3B, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0)); + + // Float + EXPECT_EQ(float_val_min, -std::numeric_limits::max()); + EXPECT_EQ(float_val_max, std::numeric_limits::max()); + + // Double + EXPECT_EQ(double_val_min, -std::numeric_limits::max()); + EXPECT_EQ(double_val_max, std::numeric_limits::max()); + + // Bit + EXPECT_EQ(bit_val_false, false); + EXPECT_EQ(bit_val_true, true); + + // Characters + EXPECT_EQ(char_val[0], 'Z'); + EXPECT_EQ(wchar_val[0], L'你'); + EXPECT_EQ(wvarchar_val[0], L'你'); + EXPECT_EQ(wvarchar_val[1], L'好'); + + EXPECT_EQ(varchar_val[0], 'X'); + EXPECT_EQ(varchar_val[1], 'Y'); + EXPECT_EQ(varchar_val[2], 'Z'); + + // Date + EXPECT_EQ(date_val_min.day, 1); + EXPECT_EQ(date_val_min.month, 1); + EXPECT_EQ(date_val_min.year, 1400); + + EXPECT_EQ(date_val_max.day, 31); + EXPECT_EQ(date_val_max.month, 12); + EXPECT_EQ(date_val_max.year, 9999); + + // Timestamp + EXPECT_EQ(timestamp_val_min.day, 1); + EXPECT_EQ(timestamp_val_min.month, 1); + EXPECT_EQ(timestamp_val_min.year, 1400); + EXPECT_EQ(timestamp_val_min.hour, 0); + EXPECT_EQ(timestamp_val_min.minute, 0); + EXPECT_EQ(timestamp_val_min.second, 0); + EXPECT_EQ(timestamp_val_min.fraction, 0); + + EXPECT_EQ(timestamp_val_max.day, 31); + EXPECT_EQ(timestamp_val_max.month, 12); + EXPECT_EQ(timestamp_val_max.year, 9999); + EXPECT_EQ(timestamp_val_max.hour, 23); + EXPECT_EQ(timestamp_val_max.minute, 59); + EXPECT_EQ(timestamp_val_max.second, 59); + EXPECT_EQ(timestamp_val_max.fraction, 0); + + this->disconnect(); +} } // namespace arrow::flight::sql::odbc From 6c0ce6a16e186feb5d60b8c9250ebb6009d51af1 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 7 Jul 2025 10:32:10 -0700 Subject: [PATCH 3/9] Add more tests for sqlbindcol --- .../flight/sql/odbc/tests/statement_test.cc | 223 ++++++++++++++---- 1 file changed, 180 insertions(+), 43 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 6e35cb1925d2..6370e56f64db 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1077,8 +1077,8 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLExecDirectFloatTruncation) { } TEST_F(FlightSQLODBCRemoteTestBase, TestSQLExecDirectNullQuery) { - // Limitation on mock test server prevents null from working properly. - // Mock server has type `DENSE_UNION` for null column data. + // Limitation on mock test server prevents null from working properly, so use remote + // server instead. Mock server has type `DENSE_UNION` for null column data. this->connect(); std::wstring wsql = L"SELECT null as null_col;"; @@ -1166,8 +1166,8 @@ TEST_F(FlightSQLODBCMockTestBase, TestSQLExecDirectTruncationQueryNullIndicator) } TEST_F(FlightSQLODBCRemoteTestBase, TestSQLExecDirectNullQueryNullIndicator) { - // Limitation on mock test server prevents null from working properly. - // Mock server has type `DENSE_UNION` for null column data. + // Limitation on mock test server prevents null from working properly, so use remote + // server instead. Mock server has type `DENSE_UNION` for null column data. this->connect(); std::wstring wsql = L"SELECT null as null_col;"; @@ -1432,42 +1432,6 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLExecDirectIgnoreInvalidBufLen) { this->disconnect(); } -//-AL- TODOs for tests -// TODO: add tests that replicate queries for 1) varibinary (mock server) -// -// and 2) time (remote server) -// TODO: add tests for indicator pointer for null data etc -// TODO: add SQLBindCol tests for row fetching - -TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColSimpleQuery) { - // -AL- TODO can perhaps delete this test later after TestSQLBindColDataQuery is done, - // this is just proof of concept - this->connect(); - - // Numeric Types - - // Signed Tiny Int - int8_t stiny_int_val; - SQLLEN buf_len = sizeof(stiny_int_val); - SQLLEN ind; - - SQLRETURN ret = - SQLBindCol(this->stmt, 1, SQL_C_STINYINT, &stiny_int_val, buf_len, &ind); - - std::wstring wsql = L"SELECT 1;"; - std::vector sql0(wsql.begin(), wsql.end()); - - ret = - SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); - EXPECT_EQ(ret, SQL_SUCCESS); - - ret = SQLFetch(this->stmt); - EXPECT_EQ(ret, SQL_SUCCESS); - - // Check that 1 is fetched - EXPECT_EQ(stiny_int_val, 1); -} - TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColDataQuery) { this->connect(); @@ -1482,7 +1446,7 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColDataQuery) { SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_STINYINT, &stiny_int_val_min, buf_len, &ind); EXPECT_EQ(ret, SQL_SUCCESS); - + ret = SQLBindCol(this->stmt, 2, SQL_C_STINYINT, &stiny_int_val_max, buf_len, &ind); EXPECT_EQ(ret, SQL_SUCCESS); @@ -1648,8 +1612,7 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColDataQuery) { std::wstring wsql = this->getQueryAllDataTypes(); std::vector sql0(wsql.begin(), wsql.end()); - ret = - SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); EXPECT_EQ(ret, SQL_SUCCESS); ret = SQLFetch(this->stmt); @@ -1753,4 +1716,178 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColDataQuery) { this->disconnect(); } +TEST_F(FlightSQLODBCRemoteTestBase, TestSQLBindColTimeQuery) { + // Mock server test is skipped due to limitation on the mock server. + // Time type from mock server does not include the fraction + this->connect(); + + SQL_TIME_STRUCT time_var_min{}; + SQL_TIME_STRUCT time_var_max{}; + SQLLEN buf_len = sizeof(time_var_min); + SQLLEN ind; + + SQLRETURN ret = + SQLBindCol(this->stmt, 1, SQL_C_TYPE_TIME, &time_var_min, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLBindCol(this->stmt, 2, SQL_C_TYPE_TIME, &time_var_max, buf_len, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + std::wstring wsql = + LR"( + SELECT CAST(TIME '00:00:00' AS TIME) AS time_min, + CAST(TIME '23:59:59' AS TIME) AS time_max; + )"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Check min values for time. + EXPECT_EQ(time_var_min.hour, 0); + EXPECT_EQ(time_var_min.minute, 0); + EXPECT_EQ(time_var_min.second, 0); + + // Check max values for time. + EXPECT_EQ(time_var_max.hour, 23); + EXPECT_EQ(time_var_max.minute, 59); + EXPECT_EQ(time_var_max.second, 59); + + this->disconnect(); +} + +TEST_F(FlightSQLODBCMockTestBase, TestSQLBindColVarbinaryQuery) { + // Have binary test on mock test base as remote test servers tend to have different + // formats for binary data + this->connect(); + + // varbinary + std::vector varbinary_val(3); + SQLLEN buf_len = varbinary_val.size(); + SQLLEN ind; + SQLRETURN ret = + SQLBindCol(this->stmt, 1, SQL_C_BINARY, &varbinary_val[0], buf_len, &ind); + + std::wstring wsql = L"SELECT X'ABCDEF' AS c_varbinary;"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Check varbinary values + EXPECT_EQ(varbinary_val[0], '\xAB'); + EXPECT_EQ(varbinary_val[1], '\xCD'); + EXPECT_EQ(varbinary_val[2], '\xEF'); + + this->disconnect(); +} + +TEST_F(FlightSQLODBCRemoteTestBase, TestSQLBindColNullQuery) { + // Limitation on mock test server prevents null from working properly, so use remote + // server instead. Mock server has type `DENSE_UNION` for null column data. + this->connect(); + + SQLINTEGER val; + SQLLEN ind; + + SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_LONG, &val, 0, &ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + std::wstring wsql = L"SELECT null as null_col;"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Verify SQL_NULL_DATA is returned for indicator + EXPECT_EQ(ind, SQL_NULL_DATA); + + this->disconnect(); +} + +TEST_F(FlightSQLODBCRemoteTestBase, TestSQLBindColNullQueryNullIndicator) { + // Limitation on mock test server prevents null from working properly, so use remote + // server instead. Mock server has type `DENSE_UNION` for null column data. + this->connect(); + + SQLINTEGER val; + + SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_LONG, &val, 0, 0); + + std::wstring wsql = L"SELECT null as null_col;"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + + EXPECT_EQ(ret, SQL_ERROR); + // Verify invalid null indicator is reported, as it is required + VerifyOdbcErrorState(SQL_HANDLE_STMT, this->stmt, error_state_22002); + + this->disconnect(); +} + +TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowFetching) { + this->connect(); + + SQLINTEGER val; + SQLLEN buf_len = sizeof(val); + SQLLEN ind; + + // Same variable will be used for column 1, the value of `val` + // should be updated after every SQLFetch call. + SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_LONG, &val, buf_len, &ind); + + std::wstring wsql = + LR"( + SELECT 1 AS small_table + UNION ALL + SELECT 2 + UNION ALL + SELECT 3; + )"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Fetch row 1 + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Verify 1 is returned + EXPECT_EQ(val, 1); + + // Fetch row 2 + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Verify 2 is returned + EXPECT_EQ(val, 2); + + // Fetch row 3 + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Verify 3 is returned + EXPECT_EQ(val, 3); + + // Verify result set has no more data beyond row 3 + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_NO_DATA); + + this->disconnect(); +} + } // namespace arrow::flight::sql::odbc From e1f42779f14418c410e238230d22797f253aee12 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 7 Jul 2025 11:25:54 -0700 Subject: [PATCH 4/9] SQL_UNBIND implementation + tests --- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 14 ++- .../flight/sql/odbc/tests/statement_test.cc | 89 +++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index d748c0f99c3f..15b26ebaf6bc 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -197,9 +197,18 @@ SQLRETURN SQLFreeStmt(SQLHSTMT handle, SQLUSMALLINT option) { return SQLFreeHandle(SQL_HANDLE_STMT, handle); } - // TODO Implement SQLBindCol + // TODO Implement SQLBindCol -AL- + // Implement SQL_UNBIND in SQLFreeStmt case SQL_UNBIND: { - return SQL_SUCCESS; + using ODBC::ODBCDescriptor; + using ODBC::ODBCStatement; + return ODBCStatement::ExecuteWithDiagnostics(handle, SQL_ERROR, [=]() { + ODBCStatement* statement = reinterpret_cast(handle); + ODBCDescriptor* ard = statement->GetARD(); + // Unbind columns + ard->SetHeaderField(SQL_DESC_COUNT, (void*)0, 0); + return SQL_SUCCESS; + }); } // SQLBindParameter is not supported @@ -985,6 +994,7 @@ SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT recordNumber, SQLSMALLINT cType using ODBC::ODBCDescriptor; using ODBC::ODBCStatement; return ODBCStatement::ExecuteWithDiagnostics(stmt, SQL_ERROR, [=]() { + // GH-47021: implement driver to return indicator value when data pointer is null ODBCStatement* statement = reinterpret_cast(stmt); ODBCDescriptor* ard = statement->GetARD(); ard->BindCol(recordNumber, cType, dataPtr, bufferLength, indicatorPtr); diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 6370e56f64db..90491206806c 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1890,4 +1890,93 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowFetching) { this->disconnect(); } +TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColIndicatorOnly) { + // GH-47021: implement driver to return indicator value when data pointer is null + GTEST_SKIP(); + // Verify driver supports null data pointer with valid indicator pointer + this->connect(); + + // Numeric Types + + // Signed Tiny Int + SQLLEN stiny_int_ind; + + SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_STINYINT, 0, 0, &stiny_int_ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Characters + SQLLEN buf_len = sizeof(SQLCHAR) * 2; + SQLLEN char_val_ind; + + ret = SQLBindCol(this->stmt, 25, SQL_C_CHAR, 0, buf_len, &char_val_ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Execute query and fetch data once since there is only 1 row. + std::wstring wsql = this->getQueryAllDataTypes(); + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Verify values for indicator pointer + // Signed Tiny Int + EXPECT_EQ(stiny_int_ind, 1); + + // Char array + EXPECT_EQ(char_val_ind, 1); + this->disconnect(); +} + +TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColIndicatorOnlySQLUnbind) { + // Verify driver supports valid indicator pointer after unbinding all columns + this->connect(); + + // Numeric Types + + // Signed Tiny Int + int8_t stiny_int_val; + SQLLEN stiny_int_ind; + + SQLRETURN ret = + SQLBindCol(this->stmt, 1, SQL_C_STINYINT, &stiny_int_val, 0, &stiny_int_ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Characters + SQLCHAR char_val[2]; + SQLLEN buf_len = sizeof(SQLCHAR) * 2; + SQLLEN char_val_ind; + + ret = SQLBindCol(this->stmt, 25, SQL_C_CHAR, &char_val, buf_len, &char_val_ind); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Driver should still be able to execute queries after unbinding columns + ret = SQLFreeStmt(this->stmt, SQL_UNBIND); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Execute query and fetch data once since there is only 1 row. + std::wstring wsql = this->getQueryAllDataTypes(); + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // GH-47021: implement driver to return indicator value when data pointer is null and + // uncomment the checks Verify values for indicator pointer Signed Tiny Int + // EXPECT_EQ(stiny_int_ind, 1); + + // Char array + // EXPECT_EQ(char_val_ind, 1); + + this->disconnect(); +} + +// TODO: -AL- Add tests for SQL_ATTR_ROW_ARRAY_SIZE +// after SQLSetStmtAttr is implemented + } // namespace arrow::flight::sql::odbc From 4a82a9f4e500770231e22363e26cdcb2666cf5d5 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 7 Jul 2025 13:30:54 -0700 Subject: [PATCH 5/9] Remove todo comments --- cpp/src/arrow/flight/sql/odbc/odbc_api.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc index 15b26ebaf6bc..8a823ee20866 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_api.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_api.cc @@ -197,8 +197,6 @@ SQLRETURN SQLFreeStmt(SQLHSTMT handle, SQLUSMALLINT option) { return SQLFreeHandle(SQL_HANDLE_STMT, handle); } - // TODO Implement SQLBindCol -AL- - // Implement SQL_UNBIND in SQLFreeStmt case SQL_UNBIND: { using ODBC::ODBCDescriptor; using ODBC::ODBCStatement; From 060815635ab4f3e3d7e69c87339893e8db416014 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 14 Jul 2025 13:44:55 -0700 Subject: [PATCH 6/9] Add tests for `SQL_ATTR_ROW_ARRAY_SIZE` --- .../flight/sql/odbc/tests/statement_test.cc | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 90491206806c..01ec84670d95 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1890,6 +1890,52 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowFetching) { this->disconnect(); } +TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowArraySize) { + this->connect(); + + constexpr SQLULEN rows = 3; + SQLINTEGER val[rows]; + SQLLEN buf_len = sizeof(val); + SQLLEN ind[rows]; + + // Same variable will be used for column 1, the value of `val` + // should be updated after every SQLFetch call. + SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_LONG, val, buf_len, ind); + + std::wstring wsql = + LR"( + SELECT 1 AS small_table + UNION ALL + SELECT 2 + UNION ALL + SELECT 3; + )"; + std::vector sql0(wsql.begin(), wsql.end()); + + ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); + EXPECT_EQ(ret, SQL_SUCCESS); + + ret = SQLSetStmtAttr(stmt, SQL_ATTR_ROW_ARRAY_SIZE, reinterpret_cast(rows), + 0); + + // Fetch 3 rows at once + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_SUCCESS); + + // Verify 1 is returned + EXPECT_EQ(val[0], 1); + // Verify 2 is returned + EXPECT_EQ(val[1], 2); + // Verify 3 is returned + EXPECT_EQ(val[2], 3); + + // Verify result set has no more data beyond row 3 + ret = SQLFetch(this->stmt); + EXPECT_EQ(ret, SQL_NO_DATA); + + this->disconnect(); +} + TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColIndicatorOnly) { // GH-47021: implement driver to return indicator value when data pointer is null GTEST_SKIP(); From 9bdbb41027062804eb0bf6c446763b9426132e17 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 14 Jul 2025 14:00:03 -0700 Subject: [PATCH 7/9] Add check for `SQL_ATTR_ROWS_FETCHED_PTR` --- cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 01ec84670d95..672495ac54cb 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1891,6 +1891,7 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowFetching) { } TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowArraySize) { + // Set SQL_ATTR_ROW_ARRAY_SIZE to fetch 3 rows at once this->connect(); constexpr SQLULEN rows = 3; @@ -1902,6 +1903,9 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowArraySize) { // should be updated after every SQLFetch call. SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_LONG, val, buf_len, ind); + SQLLEN rows_fetched; + ret = SQLSetStmtAttr(stmt, SQL_ATTR_ROWS_FETCHED_PTR, &rows_fetched, 0); + std::wstring wsql = LR"( SELECT 1 AS small_table @@ -1922,6 +1926,9 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowArraySize) { ret = SQLFetch(this->stmt); EXPECT_EQ(ret, SQL_SUCCESS); + // Verify 3 rows are fetched + EXPECT_EQ(rows_fetched, 3); + // Verify 1 is returned EXPECT_EQ(val[0], 1); // Verify 2 is returned From c14a1ef6130b7e3ea0a44b4203900e695eca602b Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 14 Jul 2025 14:03:05 -0700 Subject: [PATCH 8/9] Remove comments --- cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index 672495ac54cb..c1d0b12cd7cb 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -2028,8 +2028,4 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColIndicatorOnlySQLUnbind) { this->disconnect(); } - -// TODO: -AL- Add tests for SQL_ATTR_ROW_ARRAY_SIZE -// after SQLSetStmtAttr is implemented - } // namespace arrow::flight::sql::odbc From 4d13253858010ff5a06f702841c684306d7f9edc Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Mon, 14 Jul 2025 14:36:04 -0700 Subject: [PATCH 9/9] Fix build errors --- cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc index c1d0b12cd7cb..5585dba5dd02 100644 --- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc +++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc @@ -1904,7 +1904,7 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowArraySize) { SQLRETURN ret = SQLBindCol(this->stmt, 1, SQL_C_LONG, val, buf_len, ind); SQLLEN rows_fetched; - ret = SQLSetStmtAttr(stmt, SQL_ATTR_ROWS_FETCHED_PTR, &rows_fetched, 0); + ret = SQLSetStmtAttr(this->stmt, SQL_ATTR_ROWS_FETCHED_PTR, &rows_fetched, 0); std::wstring wsql = LR"( @@ -1919,8 +1919,8 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLBindColRowArraySize) { ret = SQLExecDirect(this->stmt, &sql0[0], static_cast(sql0.size())); EXPECT_EQ(ret, SQL_SUCCESS); - ret = SQLSetStmtAttr(stmt, SQL_ATTR_ROW_ARRAY_SIZE, reinterpret_cast(rows), - 0); + ret = SQLSetStmtAttr(this->stmt, SQL_ATTR_ROW_ARRAY_SIZE, + reinterpret_cast(rows), 0); // Fetch 3 rows at once ret = SQLFetch(this->stmt);