alinaliBQ commented on code in PR #47763:
URL: https://github.com/apache/arrow/pull/47763#discussion_r2456873604
##########
cpp/src/arrow/flight/sql/odbc/odbc_api_internal.h:
##########
@@ -27,79 +27,95 @@
//
// Define internal ODBC API function headers.
namespace arrow::flight::sql::odbc {
-SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE*
result);
-SQLRETURN SQLFreeHandle(SQLSMALLINT type, SQLHANDLE handle);
-SQLRETURN SQLFreeStmt(SQLHSTMT stmt, SQLUSMALLINT option);
-SQLRETURN SQLGetDiagField(SQLSMALLINT handle_type, SQLHANDLE handle,
- SQLSMALLINT rec_number, SQLSMALLINT diag_identifier,
- SQLPOINTER diag_info_ptr, SQLSMALLINT buffer_length,
- SQLSMALLINT* string_length_ptr);
-SQLRETURN SQLGetDiagRec(SQLSMALLINT handle_type, SQLHANDLE handle, SQLSMALLINT
rec_number,
- SQLWCHAR* sql_state, SQLINTEGER* native_error_ptr,
- SQLWCHAR* message_text, SQLSMALLINT buffer_length,
- SQLSMALLINT* text_length_ptr);
-SQLRETURN SQLGetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
- SQLINTEGER buffer_len, SQLINTEGER* str_len_ptr);
-SQLRETURN SQLSetEnvAttr(SQLHENV env, SQLINTEGER attr, SQLPOINTER value_ptr,
- SQLINTEGER str_len);
-SQLRETURN SQLGetConnectAttr(SQLHDBC conn, SQLINTEGER attribute, SQLPOINTER
value_ptr,
- SQLINTEGER buffer_length, SQLINTEGER*
string_length_ptr);
-SQLRETURN SQLSetConnectAttr(SQLHDBC conn, SQLINTEGER attr, SQLPOINTER value,
- SQLINTEGER value_len);
-SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
- SQLWCHAR* in_connection_string,
- SQLSMALLINT in_connection_string_len,
- SQLWCHAR* out_connection_string,
- SQLSMALLINT out_connection_string_buffer_len,
- SQLSMALLINT* out_connection_string_len,
- SQLUSMALLINT driver_completion);
-SQLRETURN SQLConnect(SQLHDBC conn, SQLWCHAR* dsn_name, SQLSMALLINT
dsn_name_len,
- SQLWCHAR* user_name, SQLSMALLINT user_name_len, SQLWCHAR*
password,
- SQLSMALLINT password_len);
-SQLRETURN SQLDisconnect(SQLHDBC conn);
-SQLRETURN SQLGetInfo(SQLHDBC conn, SQLUSMALLINT info_type, SQLPOINTER
info_value_ptr,
- SQLSMALLINT buf_len, SQLSMALLINT* length);
-SQLRETURN SQLGetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute, SQLPOINTER
value_ptr,
- SQLINTEGER buffer_length, SQLINTEGER*
string_length_ptr);
-SQLRETURN SQLSetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute, SQLPOINTER
value_ptr,
- SQLINTEGER stringLength);
-SQLRETURN SQLExecDirect(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER
text_length);
-SQLRETURN SQLPrepare(SQLHSTMT stmt, SQLWCHAR* queryText, SQLINTEGER
text_length);
-SQLRETURN SQLExecute(SQLHSTMT stmt);
-SQLRETURN SQLFetch(SQLHSTMT stmt);
-SQLRETURN SQLExtendedFetch(SQLHSTMT stmt, SQLUSMALLINT fetch_orientation,
- SQLLEN fetch_offset, SQLULEN* row_count_ptr,
- SQLUSMALLINT* row_status_array);
-SQLRETURN SQLFetchScroll(SQLHSTMT stmt, SQLSMALLINT fetch_orientation,
- SQLLEN fetch_offset);
-SQLRETURN SQLBindCol(SQLHSTMT stmt, SQLUSMALLINT record_number, SQLSMALLINT
c_type,
- SQLPOINTER data_ptr, SQLLEN buffer_length, SQLLEN*
indicator_ptr);
-SQLRETURN SQLCloseCursor(SQLHSTMT stmt);
-SQLRETURN SQLGetData(SQLHSTMT stmt, SQLUSMALLINT record_number, SQLSMALLINT
c_type,
- SQLPOINTER data_ptr, SQLLEN buffer_length, SQLLEN*
indicator_ptr);
-SQLRETURN SQLMoreResults(SQLHSTMT stmt);
-SQLRETURN SQLNumResultCols(SQLHSTMT stmt, SQLSMALLINT* column_count_ptr);
-SQLRETURN SQLRowCount(SQLHSTMT stmt, SQLLEN* row_count_ptr);
-SQLRETURN SQLTables(SQLHSTMT stmt, SQLWCHAR* catalog_name,
- SQLSMALLINT catalog_name_length, SQLWCHAR* schema_name,
- SQLSMALLINT schema_name_length, SQLWCHAR* table_name,
- SQLSMALLINT table_name_length, SQLWCHAR* table_type,
- SQLSMALLINT table_type_length);
-SQLRETURN SQLColumns(SQLHSTMT stmt, SQLWCHAR* catalog_name,
- SQLSMALLINT catalog_name_length, SQLWCHAR* schema_name,
- SQLSMALLINT schema_name_length, SQLWCHAR* table_name,
- SQLSMALLINT table_name_length, SQLWCHAR* column_name,
- SQLSMALLINT column_name_length);
-SQLRETURN SQLColAttribute(SQLHSTMT stmt, SQLUSMALLINT record_number,
- SQLUSMALLINT field_identifier,
- SQLPOINTER character_attribute_ptr, SQLSMALLINT
buffer_length,
- SQLSMALLINT* output_length, SQLLEN*
numeric_attribute_ptr);
-SQLRETURN SQLGetTypeInfo(SQLHSTMT stmt, SQLSMALLINT dataType);
-SQLRETURN SQLNativeSql(SQLHDBC conn, SQLWCHAR* in_statement_text,
- SQLINTEGER in_statement_text_length, SQLWCHAR*
out_statement_text,
- SQLINTEGER buffer_length, SQLINTEGER*
out_statement_text_length);
-SQLRETURN SQLDescribeCol(SQLHSTMT stmt, SQLUSMALLINT column_number, SQLWCHAR*
column_name,
- SQLSMALLINT buffer_length, SQLSMALLINT*
name_length_ptr,
- SQLSMALLINT* data_type_ptr, SQLULEN* column_size_ptr,
- SQLSMALLINT* decimal_digits_ptr, SQLSMALLINT*
nullable_ptr);
+[[nodiscard]] SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent,
Review Comment:
Added `[[nodiscard]]` here
##########
cpp/src/arrow/flight/sql/odbc/odbc_api.cc:
##########
@@ -91,8 +356,84 @@ SQLRETURN SQLGetDiagRec(SQLSMALLINT handle_type, SQLHANDLE
handle, SQLSMALLINT r
<< ", message_text: " << static_cast<const
void*>(message_text)
<< ", buffer_length: " << buffer_length
<< ", text_length_ptr: " << static_cast<const
void*>(text_length_ptr);
- // GH-46575 TODO: Implement SQLGetDiagRec
- return SQL_INVALID_HANDLE;
+ // GH-46575 TODO: Add tests for SQLGetDiagRec
+ using arrow::flight::sql::odbc::Diagnostics;
+ using ODBC::GetStringAttribute;
+ using ODBC::ODBCConnection;
+ using ODBC::ODBCDescriptor;
+ using ODBC::ODBCEnvironment;
+ using ODBC::ODBCStatement;
+
+ if (!handle) {
+ return SQL_INVALID_HANDLE;
+ }
+
+ // Record number must be greater or equal to 1
+ if (rec_number < 1 || buffer_length < 0) {
+ return SQL_ERROR;
+ }
+
+ // Set character type to be Unicode by default
+ const bool is_unicode = true;
+ Diagnostics* diagnostics = nullptr;
+
+ switch (handle_type) {
+ case SQL_HANDLE_ENV: {
+ auto* environment = ODBCEnvironment::Of(handle);
+ diagnostics = &environment->GetDiagnostics();
+ break;
+ }
+
+ case SQL_HANDLE_DBC: {
+ auto* connection = ODBCConnection::Of(handle);
+ diagnostics = &connection->GetDiagnostics();
+ break;
+ }
+
+ case SQL_HANDLE_DESC: {
+ auto* descriptor = ODBCDescriptor::Of(handle);
+ diagnostics = &descriptor->GetDiagnostics();
+ break;
+ }
+
+ case SQL_HANDLE_STMT: {
+ auto* statement = ODBCStatement::Of(handle);
+ diagnostics = &statement->GetDiagnostics();
+ break;
+ }
+
+ default:
+ return SQL_INVALID_HANDLE;
+ }
+
+ if (!diagnostics) {
+ return SQL_ERROR;
+ }
+
+ // Convert from ODBC 1 based record number to internal diagnostics 0 indexed
storage
+ const size_t record_index = static_cast<size_t>(rec_number - 1);
+ if (!diagnostics->HasRecord(record_index)) {
+ return SQL_NO_DATA;
+ }
+
+ if (sql_state) {
+ // The length of the sql state is always 5 characters plus null
+ SQLSMALLINT size = 6;
+ const std::string& state = diagnostics->GetSQLState(record_index);
+ GetStringAttribute(is_unicode, state, false, sql_state, size, &size,
*diagnostics);
Review Comment:
yup. It's not a big change so we could do it
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]