Skip to content

Commit d277098

Browse files
alinaliBQalinaliBQ
authored
Address general test code review comments
* Use RAII helper for allocating and freeing env/conn handles Avoids duplicated code Move test helper functions to anonymous namespace Add `[[nodiscard]]` for ODBC APIs Addresses community comment: https://github.com/apache/arrow/pull/47763/files#r2450014186 Remove `using List=` in test suite definitions Use static_cast for `SQLWCHAR` in type info test Use static_cast for `SQLWCHAR` in tables test * use mutable arrays for places where characters cannot be const Use static_cast for `SQLWCHAR` in columns test Update comment Use static_cast for `SQLWCHAR` in SQLGetInfo test * Address Justin's comments * Add fix for skipping the allocation of handles
1 parent 78ef45d commit d277098

14 files changed

+511
-609
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
#include "arrow/flight/sql/odbc/odbc_impl/spi/connection.h"
3232
#include "arrow/util/logging.h"
3333

34-
#if defined _WIN32 || defined _WIN64
34+
#if defined _WIN32
3535
// For displaying DSN Window
3636
# include "arrow/flight/sql/odbc/odbc_impl/system_dsn.h"
3737
#endif
@@ -810,7 +810,7 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
810810

811811
// GH-46448 TODO: Implement SQL_DRIVER_COMPLETE_REQUIRED in SQLDriverConnect according
812812
// to the spec
813-
#if defined _WIN32 || defined _WIN64
813+
#if defined _WIN32
814814
// Load the DSN window according to driver_completion
815815
if (driver_completion == SQL_DRIVER_PROMPT) {
816816
// Load DSN window before first attempt to connect

‎cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h‎

Lines changed: 91 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -27,79 +27,95 @@
2727
//
2828
// Define internal ODBC API function headers.
2929
namespace arrow::flight::sql::odbc {
30-
SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE* result);
31-
SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle);
32-
SQLRETURN SQLFreeStmt(SQLHSTMT stmt, SQLUSMALLINT option);
33-
SQLRETURN SQLGetDiagField(SQLSMALLINT handle_type, SQLHANDLE handle,
34-
SQLSMALLINT rec_number, SQLSMALLINT diag_identifier,
35-
SQLPOINTER diag_info_ptr, SQLSMALLINT buffer_length,
36-
SQLSMALLINT* string_length_ptr);
37-
SQLRETURN SQLGetDiagRec(SQLSMALLINT handle_type, SQLHANDLE handle, SQLSMALLINT rec_number,
38-
SQLWCHAR* sql_state, SQLINTEGER* native_error_ptr,
39-
SQLWCHAR* message_text, SQLSMALLINT buffer_length,
40-
SQLSMALLINT* text_length_ptr);
41-
SQLRETURN SQLGetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
42-
SQLINTEGER buffer_len, SQLINTEGER* str_len_ptr);
43-
SQLRETURN SQLSetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
44-
SQLINTEGER str_len);
45-
SQLRETURN SQLGetConnectAttr(SQLHDBC conn, SQLINTEGER attribute, SQLPOINTER value_ptr,
46-
SQLINTEGER buffer_length, SQLINTEGER* string_length_ptr);
47-
SQLRETURN SQLSetConnectAttr(SQLHDBC conn, SQLINTEGER attr, SQLPOINTER value,
48-
SQLINTEGER value_len);
49-
SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
50-
SQLWCHAR* in_connection_string,
51-
SQLSMALLINT in_connection_string_len,
52-
SQLWCHAR* out_connection_string,
53-
SQLSMALLINT out_connection_string_buffer_len,
54-
SQLSMALLINT* out_connection_string_len,
55-
SQLUSMALLINT driver_completion);
56-
SQLRETURN SQLConnect(SQLHDBC conn, SQLWCHAR* dsn_name, SQLSMALLINT dsn_name_len,
57-
SQLWCHAR* user_name, SQLSMALLINT user_name_len, SQLWCHAR* password,
58-
SQLSMALLINT password_len);
59-
SQLRETURN SQLDisconnect(SQLHDBC conn);
60-
SQLRETURN SQLGetInfo(SQLHDBC conn, SQLUSMALLINT info_type, SQLPOINTER info_value_ptr,
61-
SQLSMALLINT buf_len, SQLSMALLINT* length);
62-
SQLRETURN SQLGetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute, SQLPOINTER value_ptr,
63-
SQLINTEGER buffer_length, SQLINTEGER* string_length_ptr);
64-
SQLRETURN SQLSetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute, SQLPOINTER value_ptr,
65-
SQLINTEGER stringLength);
66-
SQLRETURN SQLExecDirect(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER text_length);
67-
SQLRETURN SQLPrepare(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER text_length);
68-
SQLRETURN SQLExecute(SQLHSTMT stmt);
69-
SQLRETURN SQLFetch(SQLHSTMT stmt);
70-
SQLRETURN SQLExtendedFetch(SQLHSTMT stmt, SQLUSMALLINT fetch_orientation,
71-
SQLLEN fetch_offset, SQLULEN* row_count_ptr,
72-
SQLUSMALLINT* row_status_array);
73-
SQLRETURN SQLFetchScroll(SQLHSTMT stmt, SQLSMALLINT fetch_orientation,
74-
SQLLEN fetch_offset);
75-
SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT record_number, SQLSMALLINT c_type,
76-
SQLPOINTER data_ptr, SQLLEN buffer_length, SQLLEN* indicator_ptr);
77-
SQLRETURN SQLCloseCursor(SQLHSTMT stmt);
78-
SQLRETURN SQLGetData(SQLHSTMT stmt, SQLUSMALLINT record_number, SQLSMALLINT c_type,
79-
SQLPOINTER data_ptr, SQLLEN buffer_length, SQLLEN* indicator_ptr);
80-
SQLRETURN SQLMoreResults(SQLHSTMT stmt);
81-
SQLRETURN SQLNumResultCols(SQLHSTMT stmt, SQLSMALLINT* column_count_ptr);
82-
SQLRETURN SQLRowCount(SQLHSTMT stmt, SQLLEN* row_count_ptr);
83-
SQLRETURN SQLTables(SQLHSTMT stmt, SQLWCHAR* catalog_name,
84-
SQLSMALLINT catalog_name_length, SQLWCHAR* schema_name,
85-
SQLSMALLINT schema_name_length, SQLWCHAR* table_name,
86-
SQLSMALLINT table_name_length, SQLWCHAR* table_type,
87-
SQLSMALLINT table_type_length);
88-
SQLRETURN SQLColumns(SQLHSTMT stmt, SQLWCHAR* catalog_name,
89-
SQLSMALLINT catalog_name_length, SQLWCHAR* schema_name,
90-
SQLSMALLINT schema_name_length, SQLWCHAR* table_name,
91-
SQLSMALLINT table_name_length, SQLWCHAR* column_name,
92-
SQLSMALLINT column_name_length);
93-
SQLRETURN SQLColAttribute(SQLHSTMT stmt, SQLUSMALLINT record_number,
94-
SQLUSMALLINT field_identifier,
95-
SQLPOINTER character_attribute_ptr, SQLSMALLINT buffer_length,
96-
SQLSMALLINT* output_length, SQLLEN* numeric_attribute_ptr);
97-
SQLRETURN SQLGetTypeInfo(SQLHSTMT stmt, SQLSMALLINT dataType);
98-
SQLRETURN SQLNativeSql(SQLHDBC conn, SQLWCHAR* in_statement_text,
99-
SQLINTEGER in_statement_text_length, SQLWCHAR* out_statement_text,
100-
SQLINTEGER buffer_length, SQLINTEGER* out_statement_text_length);
101-
SQLRETURN SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number, SQLWCHAR* column_name,
102-
SQLSMALLINT buffer_length, SQLSMALLINT* name_length_ptr,
103-
SQLSMALLINT* data_type_ptr, SQLULEN* column_size_ptr,
104-
SQLSMALLINT* decimal_digits_ptr, SQLSMALLINT* nullable_ptr);
30+
[[nodiscard]] SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent,
31+
SQLHANDLE* result);
32+
[[nodiscard]] SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle);
33+
[[nodiscard]] SQLRETURN SQLFreeStmt(SQLHSTMT stmt, SQLUSMALLINT option);
34+
[[nodiscard]] SQLRETURN SQLGetDiagField(SQLSMALLINT handle_type, SQLHANDLE handle,
35+
SQLSMALLINT rec_number,
36+
SQLSMALLINT diag_identifier,
37+
SQLPOINTER diag_info_ptr,
38+
SQLSMALLINT buffer_length,
39+
SQLSMALLINT* string_length_ptr);
40+
[[nodiscard]] SQLRETURN SQLGetDiagRec(SQLSMALLINT handle_type, SQLHANDLE handle,
41+
SQLSMALLINT rec_number, SQLWCHAR* sql_state,
42+
SQLINTEGER* native_error_ptr,
43+
SQLWCHAR* message_text, SQLSMALLINT buffer_length,
44+
SQLSMALLINT* text_length_ptr);
45+
[[nodiscard]] SQLRETURN SQLGetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
46+
SQLINTEGER buffer_len, SQLINTEGER* str_len_ptr);
47+
[[nodiscard]] SQLRETURN SQLSetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
48+
SQLINTEGER str_len);
49+
[[nodiscard]] SQLRETURN SQLGetConnectAttr(SQLHDBC conn, SQLINTEGER attribute,
50+
SQLPOINTER value_ptr, SQLINTEGER buffer_length,
51+
SQLINTEGER* string_length_ptr);
52+
[[nodiscard]] SQLRETURN SQLSetConnectAttr(SQLHDBC conn, SQLINTEGER attr, SQLPOINTER value,
53+
SQLINTEGER value_len);
54+
[[nodiscard]] SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
55+
SQLWCHAR* in_connection_string,
56+
SQLSMALLINT in_connection_string_len,
57+
SQLWCHAR* out_connection_string,
58+
SQLSMALLINT out_connection_string_buffer_len,
59+
SQLSMALLINT* out_connection_string_len,
60+
SQLUSMALLINT driver_completion);
61+
[[nodiscard]] SQLRETURN SQLConnect(SQLHDBC conn, SQLWCHAR* dsn_name,
62+
SQLSMALLINT dsn_name_len, SQLWCHAR* user_name,
63+
SQLSMALLINT user_name_len, SQLWCHAR* password,
64+
SQLSMALLINT password_len);
65+
[[nodiscard]] SQLRETURN SQLDisconnect(SQLHDBC conn);
66+
[[nodiscard]] SQLRETURN SQLGetInfo(SQLHDBC conn, SQLUSMALLINT info_type,
67+
SQLPOINTER info_value_ptr, SQLSMALLINT buf_len,
68+
SQLSMALLINT* length);
69+
[[nodiscard]] SQLRETURN SQLGetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute,
70+
SQLPOINTER value_ptr, SQLINTEGER buffer_length,
71+
SQLINTEGER* string_length_ptr);
72+
[[nodiscard]] SQLRETURN SQLSetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute,
73+
SQLPOINTER value_ptr, SQLINTEGER stringLength);
74+
[[nodiscard]] SQLRETURN SQLExecDirect(SQLHSTMT stmt, SQLWCHAR* queryText,
75+
SQLINTEGER text_length);
76+
[[nodiscard]] SQLRETURN SQLPrepare(SQLHSTMT stmt, SQLWCHAR* queryText,
77+
SQLINTEGER text_length);
78+
[[nodiscard]] SQLRETURN SQLExecute(SQLHSTMT stmt);
79+
[[nodiscard]] SQLRETURN SQLFetch(SQLHSTMT stmt);
80+
[[nodiscard]] SQLRETURN SQLExtendedFetch(SQLHSTMT stmt, SQLUSMALLINT fetch_orientation,
81+
SQLLEN fetch_offset, SQLULEN* row_count_ptr,
82+
SQLUSMALLINT* row_status_array);
83+
[[nodiscard]] SQLRETURN SQLFetchScroll(SQLHSTMT stmt, SQLSMALLINT fetch_orientation,
84+
SQLLEN fetch_offset);
85+
[[nodiscard]] SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT record_number,
86+
SQLSMALLINT c_type, SQLPOINTER data_ptr,
87+
SQLLEN buffer_length, SQLLEN* indicator_ptr);
88+
[[nodiscard]] SQLRETURN SQLCloseCursor(SQLHSTMT stmt);
89+
[[nodiscard]] SQLRETURN SQLGetData(SQLHSTMT stmt, SQLUSMALLINT record_number,
90+
SQLSMALLINT c_type, SQLPOINTER data_ptr,
91+
SQLLEN buffer_length, SQLLEN* indicator_ptr);
92+
[[nodiscard]] SQLRETURN SQLMoreResults(SQLHSTMT stmt);
93+
[[nodiscard]] SQLRETURN SQLNumResultCols(SQLHSTMT stmt, SQLSMALLINT* column_count_ptr);
94+
[[nodiscard]] SQLRETURN SQLRowCount(SQLHSTMT stmt, SQLLEN* row_count_ptr);
95+
[[nodiscard]] SQLRETURN SQLTables(SQLHSTMT stmt, SQLWCHAR* catalog_name,
96+
SQLSMALLINT catalog_name_length, SQLWCHAR* schema_name,
97+
SQLSMALLINT schema_name_length, SQLWCHAR* table_name,
98+
SQLSMALLINT table_name_length, SQLWCHAR* table_type,
99+
SQLSMALLINT table_type_length);
100+
[[nodiscard]] SQLRETURN SQLColumns(SQLHSTMT stmt, SQLWCHAR* catalog_name,
101+
SQLSMALLINT catalog_name_length, SQLWCHAR* schema_name,
102+
SQLSMALLINT schema_name_length, SQLWCHAR* table_name,
103+
SQLSMALLINT table_name_length, SQLWCHAR* column_name,
104+
SQLSMALLINT column_name_length);
105+
[[nodiscard]] SQLRETURN SQLColAttribute(SQLHSTMT stmt, SQLUSMALLINT record_number,
106+
SQLUSMALLINT field_identifier,
107+
SQLPOINTER character_attribute_ptr,
108+
SQLSMALLINT buffer_length,
109+
SQLSMALLINT* output_length,
110+
SQLLEN* numeric_attribute_ptr);
111+
[[nodiscard]] SQLRETURN SQLGetTypeInfo(SQLHSTMT stmt, SQLSMALLINT dataType);
112+
[[nodiscard]] SQLRETURN SQLNativeSql(SQLHDBC conn, SQLWCHAR* in_statement_text,
113+
SQLINTEGER in_statement_text_length,
114+
SQLWCHAR* out_statement_text,
115+
SQLINTEGER buffer_length,
116+
SQLINTEGER* out_statement_text_length);
117+
[[nodiscard]] SQLRETURN SQLDescribeCol(
118+
SQLHSTMT stmt, SQLUSMALLINT column_number, SQLWCHAR* column_name,
119+
SQLSMALLINT buffer_length, SQLSMALLINT* name_length_ptr, SQLSMALLINT* data_type_ptr,
120+
SQLULEN* column_size_ptr, SQLSMALLINT* decimal_digits_ptr, SQLSMALLINT* nullable_ptr);
105121
} // namespace arrow::flight::sql::odbc

0 commit comments

Comments
 (0)