Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/cpp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ jobs:
ARROW_DATASET: ON
ARROW_FLIGHT: ON
ARROW_FLIGHT_SQL: ON
ARROW_FLIGHT_SQL_ODBC: ON
ARROW_FLIGHT_SQL_ODBC: OFF
ARROW_GANDIVA: ON
Comment on lines 312 to 314
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in #48019, we will try to enable ODBC on MSVC, so I am removing ODBC build from MinGW here

ARROW_GCS: ON
ARROW_HDFS: OFF
Expand Down Expand Up @@ -389,10 +389,6 @@ jobs:
PIPX_BASE_PYTHON: ${{ steps.python-install.outputs.python-path }}
run: |
ci/scripts/install_gcs_testbench.sh default
- name: Register Flight SQL ODBC Driver
shell: cmd
run: |
call "cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd" ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\libarrow_flight_sql_odbc.dll
- name: Test
shell: msys2 {0}
run: |
Expand Down
3 changes: 1 addition & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc.def
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@

LIBRARY arrow_flight_sql_odbc
EXPORTS
; GH-46574 TODO enable DSN window
; ConfigDSNW
ConfigDSNW
SQLAllocConnect
SQLAllocEnv
SQLAllocHandle
Expand Down
157 changes: 147 additions & 10 deletions cpp/src/arrow/flight/sql/odbc/odbc_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
#include "arrow/flight/sql/odbc/odbc_impl/spi/connection.h"
#include "arrow/util/logging.h"

#if defined _WIN32
// For displaying DSN Window
# include "arrow/flight/sql/odbc/odbc_impl/system_dsn.h"
#endif // defined(_WIN32)

namespace arrow::flight::sql::odbc {
SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE* result) {
ARROW_LOG(DEBUG) << "SQLAllocHandle called with type: " << type
Expand Down Expand Up @@ -718,8 +723,30 @@ SQLRETURN SQLSetConnectAttr(SQLHDBC conn, SQLINTEGER attr, SQLPOINTER value_ptr,
ARROW_LOG(DEBUG) << "SQLSetConnectAttrW called with conn: " << conn
<< ", attr: " << attr << ", value_ptr: " << value_ptr
<< ", value_len: " << value_len;
// GH-47708 TODO: Implement SQLSetConnectAttr
return SQL_INVALID_HANDLE;
// GH-47708 TODO: Add tests for SQLSetConnectAttr
using ODBC::ODBCConnection;

return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() {
const bool is_unicode = true;
ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(conn);
connection->SetConnectAttr(attr, value_ptr, value_len, is_unicode);
return SQL_SUCCESS;
});
}

// Load properties from the given DSN. The properties loaded do _not_ overwrite existing
// entries in the properties.
void LoadPropertiesFromDSN(const std::string& dsn,
Connection::ConnPropertyMap& properties) {
arrow::flight::sql::odbc::config::Configuration config;
config.LoadDsn(dsn);
Connection::ConnPropertyMap dsn_properties = config.GetProperties();
for (auto& [key, value] : dsn_properties) {
auto prop_iter = properties.find(key);
if (prop_iter == properties.end()) {
properties.emplace(std::make_pair(std::move(key), std::move(value)));
}
}
}

SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
Expand All @@ -740,13 +767,73 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
<< out_connection_string_buffer_len << ", out_connection_string_len: "
<< static_cast<const void*>(out_connection_string_len)
<< ", driver_completion: " << driver_completion;

// GH-46449 TODO: Implement FILEDSN and SAVEFILE keywords according to the spec

// GH-46560 TODO: Copy connection string properly in SQLDriverConnect according to the
// spec

// GH-46574 TODO: Implement SQLDriverConnect
return SQL_INVALID_HANDLE;
using ODBC::ODBCConnection;

return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() {
ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(conn);
std::string connection_string =
ODBC::SqlWcharToString(in_connection_string, in_connection_string_len);
Connection::ConnPropertyMap properties;
std::string dsn_value = "";
std::optional<std::string> dsn = ODBCConnection::GetDsnIfExists(connection_string);
if (dsn.has_value()) {
dsn_value = dsn.value();
LoadPropertiesFromDSN(dsn_value, properties);
}
ODBCConnection::GetPropertiesFromConnString(connection_string, properties);

std::vector<std::string_view> missing_properties;

// GH-46448 TODO: Implement SQL_DRIVER_COMPLETE_REQUIRED in SQLDriverConnect according
// to the spec
#if defined _WIN32
// Load the DSN window according to driver_completion
if (driver_completion == SQL_DRIVER_PROMPT) {
// Load DSN window before first attempt to connect
arrow::flight::sql::odbc::config::Configuration config;
if (!DisplayConnectionWindow(window_handle, config, properties)) {
return static_cast<SQLRETURN>(SQL_NO_DATA);
}
connection->Connect(dsn_value, properties, missing_properties);
} else if (driver_completion == SQL_DRIVER_COMPLETE ||
driver_completion == SQL_DRIVER_COMPLETE_REQUIRED) {
try {
connection->Connect(dsn_value, properties, missing_properties);
} catch (const DriverException&) {
// If first connection fails due to missing attributes, load
// the DSN window and try to connect again
if (!missing_properties.empty()) {
arrow::flight::sql::odbc::config::Configuration config;
missing_properties.clear();

if (!DisplayConnectionWindow(window_handle, config, properties)) {
return static_cast<SQLRETURN>(SQL_NO_DATA);
}
connection->Connect(dsn_value, properties, missing_properties);
} else {
throw;
}
}
} else {
// Default case: attempt connection without showing DSN window
connection->Connect(dsn_value, properties, missing_properties);
}
#else
// Attempt connection without loading DSN window on macOS/Linux
connection->Connect(dsn, properties, missing_properties);
#endif
// Copy connection string to out_connection_string after connection attempt
return ODBC::GetStringAttribute(true, connection_string, false, out_connection_string,
out_connection_string_buffer_len,
out_connection_string_len,
connection->GetDiagnostics());
});
}

SQLRETURN SQLConnect(SQLHDBC conn, SQLWCHAR* dsn_name, SQLSMALLINT dsn_name_len,
Expand All @@ -759,14 +846,48 @@ SQLRETURN SQLConnect(SQLHDBC conn, SQLWCHAR* dsn_name, SQLSMALLINT dsn_name_len,
<< ", user_name_len: " << user_name_len
<< ", password: " << static_cast<const void*>(password)
<< ", password_len: " << password_len;
// GH-46574 TODO: Implement SQLConnect
return SQL_INVALID_HANDLE;

using ODBC::ODBCConnection;

using ODBC::SqlWcharToString;

return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() {
ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(conn);
std::string dsn = SqlWcharToString(dsn_name, dsn_name_len);

Configuration config;
config.LoadDsn(dsn);

if (user_name) {
std::string uid = SqlWcharToString(user_name, user_name_len);
config.Emplace(FlightSqlConnection::UID, std::move(uid));
}

if (password) {
std::string pwd = SqlWcharToString(password, password_len);
config.Emplace(FlightSqlConnection::PWD, std::move(pwd));
}

std::vector<std::string_view> missing_properties;

connection->Connect(dsn, config.GetProperties(), missing_properties);

return SQL_SUCCESS;
});
}

SQLRETURN SQLDisconnect(SQLHDBC conn) {
ARROW_LOG(DEBUG) << "SQLDisconnect called with conn: " << conn;
// GH-46574 TODO: Implement SQLDisconnect
return SQL_INVALID_HANDLE;

using ODBC::ODBCConnection;

return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() {
ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(conn);

connection->Disconnect();

return SQL_SUCCESS;
});
}

SQLRETURN SQLGetInfo(SQLHDBC conn, SQLUSMALLINT info_type, SQLPOINTER info_value_ptr,
Expand All @@ -776,8 +897,24 @@ SQLRETURN SQLGetInfo(SQLHDBC conn, SQLUSMALLINT info_type, SQLPOINTER info_value
<< ", info_value_ptr: " << info_value_ptr << ", buf_len: " << buf_len
<< ", string_length_ptr: "
<< static_cast<const void*>(string_length_ptr);
// GH-47709 TODO: Implement SQLGetInfo
return SQL_INVALID_HANDLE;

// GH-47709 TODO: Update SQLGetInfo implementation and add tests for SQLGetInfo
using ODBC::ODBCConnection;

return ODBCConnection::ExecuteWithDiagnostics(conn, SQL_ERROR, [=]() {
ODBCConnection* connection = reinterpret_cast<ODBCConnection*>(conn);

// Set character type to be Unicode by default
const bool is_unicode = true;

if (!info_value_ptr && !string_length_ptr) {
return static_cast<SQLRETURN>(SQL_ERROR);
}

connection->GetInfo(info_type, info_value_ptr, buf_len, string_length_ptr,
is_unicode);
return static_cast<SQLRETURN>(SQL_SUCCESS);
});
}

SQLRETURN SQLGetStmtAttr(SQLHSTMT stmt, SQLINTEGER attribute, SQLPOINTER value_ptr,
Expand Down
4 changes: 3 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ if(WIN32)
ui/dsn_configuration_window.h
ui/window.cc
ui/window.h
system_dsn.cc)
win_system_dsn.cc
system_dsn.cc
system_dsn.h)
endif()

target_link_libraries(arrow_odbc_spi_impl PUBLIC arrow_flight_sql_shared
Expand Down
22 changes: 11 additions & 11 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/attribute_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@

#pragma once

#include <arrow/flight/sql/odbc/odbc_impl/diagnostics.h>
#include <arrow/flight/sql/odbc/odbc_impl/exceptions.h>
#include <arrow/flight/sql/odbc/odbc_impl/platform.h>
#include <sql.h>
#include <sqlext.h>
#include <algorithm>
#include <cstring>
#include <memory>
#include "arrow/flight/sql/odbc/odbc_impl/diagnostics.h"
#include "arrow/flight/sql/odbc/odbc_impl/encoding_utils.h"
#include "arrow/flight/sql/odbc/odbc_impl/exceptions.h"
#include "arrow/flight/sql/odbc/odbc_impl/platform.h"

#include <arrow/flight/sql/odbc/odbc_impl/encoding_utils.h>

// GH-48083 TODO: replace `namespace ODBC` with `namespace arrow::flight::sql::odbc`
namespace ODBC {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(maybe for a separate issue but) can we fix this namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea we can do it for a separate issue. Are we thinking of replacing ODBC with namespace arrow::flight::sql::odbc?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created issue #48083, @justing-bq please link this issue in the code, thanks!

GH-48083 TODO: replace `namespace ODBC` with `namespace arrow::flight::sql::odbc`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a link to the github issue.


using arrow::flight::sql::odbc::Diagnostics;
Expand All @@ -48,12 +48,12 @@ inline void GetAttribute(T attribute_value, SQLPOINTER output, O output_size,
}

template <typename O>
inline SQLRETURN GetAttributeUTF8(const std::string& attribute_value, SQLPOINTER output,
inline SQLRETURN GetAttributeUTF8(std::string_view attribute_value, SQLPOINTER output,
O output_size, O* output_len_ptr) {
if (output) {
size_t output_len_before_null =
std::min(static_cast<O>(attribute_value.size()), static_cast<O>(output_size - 1));
memcpy(output, attribute_value.c_str(), output_len_before_null);
std::memcpy(output, attribute_value.data(), output_len_before_null);
reinterpret_cast<char*>(output)[output_len_before_null] = '\0';
}

Expand All @@ -68,7 +68,7 @@ inline SQLRETURN GetAttributeUTF8(const std::string& attribute_value, SQLPOINTER
}

template <typename O>
inline SQLRETURN GetAttributeUTF8(const std::string& attribute_value, SQLPOINTER output,
inline SQLRETURN GetAttributeUTF8(std::string_view attribute_value, SQLPOINTER output,
O output_size, O* output_len_ptr,
Diagnostics& diagnostics) {
SQLRETURN result =
Expand All @@ -80,7 +80,7 @@ inline SQLRETURN GetAttributeUTF8(const std::string& attribute_value, SQLPOINTER
}

template <typename O>
inline SQLRETURN GetAttributeSQLWCHAR(const std::string& attribute_value,
inline SQLRETURN GetAttributeSQLWCHAR(std::string_view attribute_value,
bool is_length_in_bytes, SQLPOINTER output,
O output_size, O* output_len_ptr) {
size_t length = ConvertToSqlWChar(
Expand All @@ -104,7 +104,7 @@ inline SQLRETURN GetAttributeSQLWCHAR(const std::string& attribute_value,
}

template <typename O>
inline SQLRETURN GetAttributeSQLWCHAR(const std::string& attribute_value,
inline SQLRETURN GetAttributeSQLWCHAR(std::string_view attribute_value,
bool is_length_in_bytes, SQLPOINTER output,
O output_size, O* output_len_ptr,
Diagnostics& diagnostics) {
Expand All @@ -117,7 +117,7 @@ inline SQLRETURN GetAttributeSQLWCHAR(const std::string& attribute_value,
}

template <typename O>
inline SQLRETURN GetStringAttribute(bool is_unicode, const std::string& attribute_value,
inline SQLRETURN GetStringAttribute(bool is_unicode, std::string_view attribute_value,
bool is_length_in_bytes, SQLPOINTER output,
O output_size, O* output_len_ptr,
Diagnostics& diagnostics) {
Expand Down
21 changes: 14 additions & 7 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/config/configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static const char DEFAULT_USE_CERT_STORE[] = TRUE_STR;
static const char DEFAULT_DISABLE_CERT_VERIFICATION[] = FALSE_STR;

namespace {
std::string ReadDsnString(const std::string& dsn, const std::string_view& key,
std::string ReadDsnString(const std::string& dsn, std::string_view key,
const std::string& dflt = "") {
CONVERT_WIDE_STR(const std::wstring wdsn, dsn);
CONVERT_WIDE_STR(const std::wstring wkey, key);
Expand Down Expand Up @@ -150,11 +150,11 @@ void Configuration::LoadDsn(const std::string& dsn) {

void Configuration::Clear() { this->properties_.clear(); }

bool Configuration::IsSet(const std::string_view& key) const {
bool Configuration::IsSet(std::string_view key) const {
return 0 != this->properties_.count(key);
}

const std::string& Configuration::Get(const std::string_view& key) const {
const std::string& Configuration::Get(std::string_view key) const {
const auto itr = this->properties_.find(key);
if (itr == this->properties_.cend()) {
static const std::string empty("");
Expand All @@ -163,15 +163,22 @@ const std::string& Configuration::Get(const std::string_view& key) const {
return itr->second;
}

void Configuration::Set(const std::string_view& key, const std::wstring& wvalue) {
void Configuration::Set(std::string_view key, const std::wstring& wvalue) {
CONVERT_UTF8_STR(const std::string value, wvalue);
Set(key, value);
}

void Configuration::Set(const std::string_view& key, const std::string& value) {
void Configuration::Set(std::string_view key, const std::string& value) {
const std::string copy = boost::trim_copy(value);
if (!copy.empty()) {
this->properties_[key] = value;
this->properties_[std::string(key)] = value;
}
}

void Configuration::Emplace(std::string_view key, std::string&& value) {
const std::string copy = boost::trim_copy(value);
if (!copy.empty()) {
this->properties_.emplace(std::make_pair(key, std::move(value)));
}
}

Expand All @@ -182,7 +189,7 @@ const Connection::ConnPropertyMap& Configuration::GetProperties() const {
std::vector<std::string_view> Configuration::GetCustomKeys() const {
Connection::ConnPropertyMap copy_props(properties_);
for (auto& key : FlightSqlConnection::ALL_KEYS) {
copy_props.erase(key);
copy_props.erase(std::string(key));
}
std::vector<std::string_view> keys;
boost::copy(copy_props | boost::adaptors::map_keys, std::back_inserter(keys));
Expand Down
Loading
Loading