From 602fea4ac881c56b3a82123e78a4080468cb19b6 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" <96995091+alinaliBQ@users.noreply.github.com> Date: Tue, 23 Sep 2025 15:38:02 -0700 Subject: [PATCH 1/6] Replace `spdlogs` with Arrow's Internal Logging * Add logging README * register kernel function conditionally * Resolves errors of kernel function already registered Logging files are not supported since GLOG is not enabled on Windows in Arrow repo. We can work + test on log file support on macOS/Linux phase * Added minor changes to fix the build --- cpp/src/arrow/flight/sql/odbc/CMakeLists.txt | 5 ++ .../sql/odbc/flight_sql/flight_sql_driver.cc | 86 +++++++------------ .../flight_sql/ui/dsn_configuration_window.cc | 2 +- .../sql/odbc/odbcabstraction/CMakeLists.txt | 21 ----- .../include/odbcabstraction/logger.h | 67 --------------- .../include/odbcabstraction/spd_logger.h | 54 ------------ .../include/odbcabstraction/utils.h | 7 +- .../flight/sql/odbc/odbcabstraction/logger.cc | 32 ------- .../sql/odbc/odbcabstraction/spd_logger.cc | 70 --------------- .../flight/sql/odbc/odbcabstraction/utils.cc | 37 -------- cpp/vcpkg.json | 1 - 11 files changed, 37 insertions(+), 345 deletions(-) delete mode 100644 cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h delete mode 100644 cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h delete mode 100644 cpp/src/arrow/flight/sql/odbc/odbcabstraction/logger.cc delete mode 100644 cpp/src/arrow/flight/sql/odbc/odbcabstraction/spd_logger.cc diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index 80be0dee99f..1d18b1590cb 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -17,5 +17,10 @@ add_custom_target(arrow_flight_sql_odbc) +# Use C++ 20 for ODBC and its subdirectory +# GH-44792: Arrow will switch to C++ 20 +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + add_subdirectory(flight_sql) add_subdirectory(odbcabstraction) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc index 1949d2f15ad..7df3f4613ae 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc @@ -18,41 +18,45 @@ #include "arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h" #include "arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h" #include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h" -#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h" -#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h" +#include "arrow/util/io_util.h" +#include "arrow/util/logging.h" -#define DEFAULT_MAXIMUM_FILE_SIZE 16777216 -#define CONFIG_FILE_NAME "arrow-odbc.ini" +#define ODBC_LOG_LEVEL "ARROW_ODBC_LOG_LEVEL" -namespace driver { -namespace flight_sql { - -using odbcabstraction::Connection; -using odbcabstraction::LogLevel; -using odbcabstraction::OdbcVersion; -using odbcabstraction::SPDLogger; +using arrow::util::ArrowLogLevel; namespace { -LogLevel ToLogLevel(int64_t level) { +/// Return the corresponding ArrowLogLevel. Debug level is returned by default. +ArrowLogLevel ToLogLevel(int64_t level) { switch (level) { + case -2: + return ArrowLogLevel::ARROW_TRACE; + case -1: + return ArrowLogLevel::ARROW_DEBUG; case 0: - return LogLevel::LogLevel_TRACE; + return ArrowLogLevel::ARROW_INFO; case 1: - return LogLevel::LogLevel_DEBUG; + return ArrowLogLevel::ARROW_WARNING; case 2: - return LogLevel::LogLevel_INFO; + return ArrowLogLevel::ARROW_ERROR; case 3: - return LogLevel::LogLevel_WARN; - case 4: - return LogLevel::LogLevel_ERROR; + return ArrowLogLevel::ARROW_FATAL; default: - return LogLevel::LogLevel_OFF; + return ArrowLogLevel::ARROW_DEBUG; } } } // namespace +namespace driver { +namespace flight_sql { + +using odbcabstraction::Connection; +using odbcabstraction::OdbcVersion; + FlightSqlDriver::FlightSqlDriver() - : diagnostics_("Apache Arrow", "Flight SQL", OdbcVersion::V_3), version_("0.9.0.0") {} + : diagnostics_("Apache Arrow", "Flight SQL", OdbcVersion::V_3), version_("0.9.0.0") { + RegisterLog(); +} std::shared_ptr FlightSqlDriver::CreateConnection(OdbcVersion odbc_version) { return std::make_shared(odbc_version, version_); @@ -63,45 +67,15 @@ odbcabstraction::Diagnostics& FlightSqlDriver::GetDiagnostics() { return diagnos void FlightSqlDriver::SetVersion(std::string version) { version_ = std::move(version); } void FlightSqlDriver::RegisterLog() { - odbcabstraction::PropertyMap propertyMap; - driver::odbcabstraction::ReadConfigFile(propertyMap, CONFIG_FILE_NAME); - - auto log_enable_iterator = propertyMap.find(SPDLogger::LOG_ENABLED); - auto log_enabled = log_enable_iterator != propertyMap.end() - ? odbcabstraction::AsBool(log_enable_iterator->second) - : false; - if (!log_enabled) { - return; - } - - auto log_path_iterator = propertyMap.find(SPDLogger::LOG_PATH); - auto log_path = log_path_iterator != propertyMap.end() ? log_path_iterator->second : ""; - if (log_path.empty()) { + std::string log_level_str = arrow::internal::GetEnvVar(ODBC_LOG_LEVEL).ValueOr(""); + if (log_level_str.empty()) { return; } + auto log_level = ToLogLevel(std::stoi(log_level_str)); - auto log_level_iterator = propertyMap.find(SPDLogger::LOG_LEVEL); - auto log_level = ToLogLevel(log_level_iterator != propertyMap.end() - ? std::stoi(log_level_iterator->second) - : 1); - if (log_level == odbcabstraction::LogLevel_OFF) { - return; - } - - auto maximum_file_size_iterator = propertyMap.find(SPDLogger::MAXIMUM_FILE_SIZE); - auto maximum_file_size = maximum_file_size_iterator != propertyMap.end() - ? std::stoi(maximum_file_size_iterator->second) - : DEFAULT_MAXIMUM_FILE_SIZE; - - auto maximum_file_quantity_iterator = propertyMap.find(SPDLogger::FILE_QUANTITY); - auto maximum_file_quantity = maximum_file_quantity_iterator != propertyMap.end() - ? std::stoi(maximum_file_quantity_iterator->second) - : 1; - - std::unique_ptr logger(new odbcabstraction::SPDLogger()); - - logger->init(maximum_file_quantity, maximum_file_size, log_path, log_level); - odbcabstraction::Logger::SetInstance(std::move(logger)); + // Enable driver logging. Log files are not supported on Windows yet, since GLOG is not + // tested fully on Windows. + arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); } } // namespace flight_sql diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/ui/dsn_configuration_window.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/ui/dsn_configuration_window.cc index 42741c5a3e5..77927734471 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/ui/dsn_configuration_window.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/ui/dsn_configuration_window.cc @@ -280,7 +280,7 @@ int DsnConfigurationWindow::CreateEncryptionSettingsGroup(int posX, int posY, in rightCheckPosX, rowPos - 2, 20, 2 * ROW_HEIGHT, "", ChildId::DISABLE_CERT_VERIFICATION_CHECKBOX, disableCertVerification); - rowPos += INTERVAL + static_cast(1.5 * ROW_HEIGHT); + rowPos += INTERVAL + static_cast(1.5 * static_cast(ROW_HEIGHT)); encryptionSettingsGroupBox = CreateGroupBox(posX, posY, sizeX, rowPos - posY, "Encryption settings", diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/CMakeLists.txt index c9614b88a5b..17abe2c1462 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/CMakeLists.txt @@ -25,9 +25,7 @@ add_library(odbcabstraction include/odbcabstraction/diagnostics.h include/odbcabstraction/error_codes.h include/odbcabstraction/exceptions.h - include/odbcabstraction/logger.h include/odbcabstraction/platform.h - include/odbcabstraction/spd_logger.h include/odbcabstraction/types.h include/odbcabstraction/utils.h include/odbcabstraction/odbc_impl/attribute_utils.h @@ -47,8 +45,6 @@ add_library(odbcabstraction diagnostics.cc encoding.cc exceptions.cc - logger.cc - spd_logger.cc utils.cc ../../../../vendored/whereami/whereami.cc odbc_impl/odbc_connection.cc @@ -65,20 +61,3 @@ set_target_properties(odbcabstraction ${CMAKE_BINARY_DIR}/$/lib RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/$/lib) - -include(FetchContent) -fetchcontent_declare(spdlog - URL https://github.com/gabime/spdlog/archive/76fb40d95455f249bd70824ecfcae7a8f0930fa3.zip - CONFIGURE_COMMAND - "" - BUILD_COMMAND - "") -fetchcontent_getproperties(spdlog) -if(NOT spdlog_POPULATED) - fetchcontent_populate(spdlog) -endif() - -add_library(spdlog INTERFACE) -target_include_directories(spdlog INTERFACE ${spdlog_SOURCE_DIR}/include) - -target_link_libraries(odbcabstraction PUBLIC spdlog) diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h deleted file mode 100644 index 5f8619cbb92..00000000000 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h +++ /dev/null @@ -1,67 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#pragma once - -#include -#include - -#include - -#define __LAZY_LOG(LEVEL, ...) \ - do { \ - driver::odbcabstraction::Logger* logger = \ - driver::odbcabstraction::Logger::GetInstance(); \ - if (logger) { \ - logger->log(driver::odbcabstraction::LogLevel::LogLevel_##LEVEL, \ - [&]() { return fmt::format(__VA_ARGS__); }); \ - } \ - } while (0) -#define LOG_DEBUG(...) __LAZY_LOG(DEBUG, __VA_ARGS__) -#define LOG_INFO(...) __LAZY_LOG(INFO, __VA_ARGS__) -#define LOG_ERROR(...) __LAZY_LOG(ERROR, __VA_ARGS__) -#define LOG_TRACE(...) __LAZY_LOG(TRACE, __VA_ARGS__) -#define LOG_WARN(...) __LAZY_LOG(WARN, __VA_ARGS__) - -namespace driver { -namespace odbcabstraction { - -enum LogLevel { - LogLevel_TRACE, - LogLevel_DEBUG, - LogLevel_INFO, - LogLevel_WARN, - LogLevel_ERROR, - LogLevel_OFF -}; - -class Logger { - protected: - Logger() = default; - - public: - static Logger* GetInstance(); - static void SetInstance(std::unique_ptr logger); - - virtual ~Logger() = default; - - virtual void log(LogLevel level, - const std::function& build_message) = 0; -}; - -} // namespace odbcabstraction -} // namespace driver diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h deleted file mode 100644 index 08672b9e7c2..00000000000 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spd_logger.h +++ /dev/null @@ -1,54 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#pragma once - -#include "odbcabstraction/logger.h" - -#include -#include - -#include - -namespace driver { -namespace odbcabstraction { - -class SPDLogger : public Logger { - protected: - std::shared_ptr logger_; - - public: - static constexpr std::string_view LOG_LEVEL = "LogLevel"; - static constexpr std::string_view LOG_PATH = "LogPath"; - static constexpr std::string_view MAXIMUM_FILE_SIZE = "MaximumFileSize"; - static constexpr std::string_view FILE_QUANTITY = "FileQuantity"; - static constexpr std::string_view LOG_ENABLED = "LogEnabled"; - - SPDLogger() = default; - ~SPDLogger() = default; - SPDLogger(SPDLogger& other) = delete; - - void operator=(const SPDLogger&) = delete; - void init(int64_t fileQuantity, int64_t maxFileSize, const std::string& fileNamePrefix, - LogLevel level); - - void log(LogLevel level, - const std::function& build_message) override; -}; - -} // namespace odbcabstraction -} // namespace driver diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h index cc848baa0fd..5e541a1d45f 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h +++ b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h @@ -17,10 +17,8 @@ #pragma once -#include -#include -#include #include +#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spi/connection.h" namespace driver { namespace odbcabstraction { @@ -51,8 +49,5 @@ boost::optional AsBool(const Connection::ConnPropertyMap& connPropertyMap, boost::optional AsInt32(int32_t min_value, const Connection::ConnPropertyMap& connPropertyMap, const std::string_view& property_name); - -void ReadConfigFile(PropertyMap& properties, const std::string& configFileName); - } // namespace odbcabstraction } // namespace driver diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/logger.cc b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/logger.cc deleted file mode 100644 index edace64cf6a..00000000000 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/logger.cc +++ /dev/null @@ -1,32 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#include - -namespace driver { -namespace odbcabstraction { - -static std::unique_ptr odbc_logger_ = nullptr; - -Logger* Logger::GetInstance() { return odbc_logger_.get(); } - -void Logger::SetInstance(std::unique_ptr logger) { - odbc_logger_ = std::move(logger); -} - -} // namespace odbcabstraction -} // namespace driver diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/spd_logger.cc b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/spd_logger.cc deleted file mode 100644 index 322ae5e5da1..00000000000 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/spd_logger.cc +++ /dev/null @@ -1,70 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#include "odbcabstraction/spd_logger.h" - -#include "odbcabstraction/logger.h" - -#include -#include -#include - -#include - -namespace driver { -namespace odbcabstraction { -namespace { -inline spdlog::level::level_enum ToSpdLogLevel(LogLevel level) { - switch (level) { - case LogLevel_TRACE: - return spdlog::level::trace; - case LogLevel_DEBUG: - return spdlog::level::debug; - case LogLevel_INFO: - return spdlog::level::info; - case LogLevel_WARN: - return spdlog::level::warn; - case LogLevel_ERROR: - return spdlog::level::err; - default: - return spdlog::level::off; - } -} -} // namespace - -void SPDLogger::init(int64_t fileQuantity, int64_t maxFileSize, - const std::string& fileNamePrefix, LogLevel level) { - logger_ = spdlog::rotating_logger_mt( - "ODBC Logger", fileNamePrefix, maxFileSize, fileQuantity); - - logger_->set_level(ToSpdLogLevel(level)); -} - -void SPDLogger::log(LogLevel level, - const std::function& build_message) { - auto level_set = logger_->level(); - spdlog::level::level_enum spdlog_level = ToSpdLogLevel(level); - if (level_set == spdlog::level::off || level_set > spdlog_level) { - return; - } - - const std::string& message = build_message(); - logger_->log(spdlog_level, message); -} - -} // namespace odbcabstraction -} // namespace driver diff --git a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/utils.cc b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/utils.cc index f1d2d14744d..d76b341f127 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbcabstraction/utils.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbcabstraction/utils.cc @@ -19,13 +19,7 @@ #include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h" -#include -#include - #include -#include -#include -#include namespace driver { namespace odbcabstraction { @@ -81,36 +75,5 @@ std::string GetModulePath() { return std::string(path.begin(), path.begin() + dirname_length); } -void ReadConfigFile(PropertyMap& properties, const std::string& config_file_name) { - auto config_path = GetModulePath(); - - std::ifstream config_file; - auto config_file_path = config_path + "/" + config_file_name; - config_file.open(config_file_path); - - if (config_file.fail()) { - auto error_msg = "Arrow Flight SQL ODBC driver config file not found on \"" + - config_file_path + "\""; - std::cerr << error_msg << std::endl; - - throw DriverException(error_msg); - } - - std::string temp_config; - - boost::char_separator separator("="); - while (config_file.good()) { - config_file >> temp_config; - boost::tokenizer> tokenizer(temp_config, separator); - - auto iterator = tokenizer.begin(); - - std::string key = *iterator; - std::string value = *++iterator; - - properties[key] = std::move(value); - } -} - } // namespace odbcabstraction } // namespace driver diff --git a/cpp/vcpkg.json b/cpp/vcpkg.json index 95e6ac83071..68f20663b59 100644 --- a/cpp/vcpkg.json +++ b/cpp/vcpkg.json @@ -54,7 +54,6 @@ "rapidjson", "re2", "snappy", - "spdlog", "sqlite3", "thrift", "utf8proc", From 74594cfb891134af419cef72207ae3aad4af5f77 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Thu, 25 Sep 2025 11:27:06 -0700 Subject: [PATCH 2/6] Address comments from Kou --- cpp/src/arrow/flight/sql/odbc/CMakeLists.txt | 1 - .../sql/odbc/flight_sql/flight_sql_driver.cc | 48 +++++++++---------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt index 1d18b1590cb..852ffae15f4 100644 --- a/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/odbc/CMakeLists.txt @@ -20,7 +20,6 @@ add_custom_target(arrow_flight_sql_odbc) # Use C++ 20 for ODBC and its subdirectory # GH-44792: Arrow will switch to C++ 20 set(CMAKE_CXX_STANDARD 20) -set(CMAKE_CXX_STANDARD_REQUIRED ON) add_subdirectory(flight_sql) add_subdirectory(odbcabstraction) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc index 7df3f4613ae..5a5b5cbda35 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc @@ -20,35 +20,13 @@ #include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h" #include "arrow/util/io_util.h" #include "arrow/util/logging.h" - -#define ODBC_LOG_LEVEL "ARROW_ODBC_LOG_LEVEL" +#include "arrow/util/string.h" using arrow::util::ArrowLogLevel; -namespace { -/// Return the corresponding ArrowLogLevel. Debug level is returned by default. -ArrowLogLevel ToLogLevel(int64_t level) { - switch (level) { - case -2: - return ArrowLogLevel::ARROW_TRACE; - case -1: - return ArrowLogLevel::ARROW_DEBUG; - case 0: - return ArrowLogLevel::ARROW_INFO; - case 1: - return ArrowLogLevel::ARROW_WARNING; - case 2: - return ArrowLogLevel::ARROW_ERROR; - case 3: - return ArrowLogLevel::ARROW_FATAL; - default: - return ArrowLogLevel::ARROW_DEBUG; - } -} -} // namespace - namespace driver { namespace flight_sql { +static constexpr const char* kODBCLogLevel = "ARROW_ODBC_LOG_LEVEL"; using odbcabstraction::Connection; using odbcabstraction::OdbcVersion; @@ -67,11 +45,29 @@ odbcabstraction::Diagnostics& FlightSqlDriver::GetDiagnostics() { return diagnos void FlightSqlDriver::SetVersion(std::string version) { version_ = std::move(version); } void FlightSqlDriver::RegisterLog() { - std::string log_level_str = arrow::internal::GetEnvVar(ODBC_LOG_LEVEL).ValueOr(""); + std::string log_level_str = arrow::internal::GetEnvVar(kODBCLogLevel) + .Map(arrow::internal::AsciiToLower) + .Map(arrow::internal::TrimString) + .ValueOr(""); if (log_level_str.empty()) { return; } - auto log_level = ToLogLevel(std::stoi(log_level_str)); + + auto log_level = ArrowLogLevel::ARROW_DEBUG; + + if (log_level_str == "fatal") { + log_level = ArrowLogLevel::ARROW_FATAL; + } else if (log_level_str == "error") { + log_level = ArrowLogLevel::ARROW_ERROR; + } else if (log_level_str == "warning") { + log_level = ArrowLogLevel::ARROW_WARNING; + } else if (log_level_str == "info") { + log_level = ArrowLogLevel::ARROW_INFO; + } else if (log_level_str == "debug") { + log_level = ArrowLogLevel::ARROW_DEBUG; + } else if (log_level_str == "trace") { + log_level = ArrowLogLevel::ARROW_TRACE; + } // Enable driver logging. Log files are not supported on Windows yet, since GLOG is not // tested fully on Windows. From 90d0a72670f81721e2e3e228e297c2c934e370a5 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 26 Sep 2025 14:21:16 -0700 Subject: [PATCH 3/6] Call `ShutDownArrowLog` --- .../arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc | 8 ++++++++ .../flight_sql/include/flight_sql/flight_sql_driver.h | 1 + 2 files changed, 9 insertions(+) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc index 5a5b5cbda35..7481adc3f06 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc @@ -36,6 +36,14 @@ FlightSqlDriver::FlightSqlDriver() RegisterLog(); } +FlightSqlDriver::~FlightSqlDriver() { + // Unregister log if logging is enabled + if (arrow::internal::GetEnvVar(kODBCLogLevel).ValueOr("").empty()) { + return; + } + arrow::util::ArrowLog::ShutDownArrowLog(); +} + std::shared_ptr FlightSqlDriver::CreateConnection(OdbcVersion odbc_version) { return std::make_shared(odbc_version, version_); } diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h b/cpp/src/arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h index 88460cdf5b2..f349fa3344b 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/include/flight_sql/flight_sql_driver.h @@ -30,6 +30,7 @@ class FlightSqlDriver : public odbcabstraction::Driver { public: FlightSqlDriver(); + ~FlightSqlDriver(); std::shared_ptr CreateConnection( odbcabstraction::OdbcVersion odbc_version) override; From 93f095cebded9f75963932653925c632b544e2ee Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 26 Sep 2025 16:51:39 -0700 Subject: [PATCH 4/6] Register log confditionally --- cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc index 7481adc3f06..cb8c1f86859 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc @@ -79,7 +79,9 @@ void FlightSqlDriver::RegisterLog() { // Enable driver logging. Log files are not supported on Windows yet, since GLOG is not // tested fully on Windows. - arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); + if (!arrow::util::ArrowLog::IsLevelEnabled(log_level)) { + arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); + } } } // namespace flight_sql From 116ae2d5aa9fb24edcd22c4d6c5c7d80a5bb2e2c Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 26 Sep 2025 17:10:57 -0700 Subject: [PATCH 5/6] Revert "Register log confditionally" This reverts commit 93f095cebded9f75963932653925c632b544e2ee. The logic is flawed. --- cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc index cb8c1f86859..7481adc3f06 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc @@ -79,9 +79,7 @@ void FlightSqlDriver::RegisterLog() { // Enable driver logging. Log files are not supported on Windows yet, since GLOG is not // tested fully on Windows. - if (!arrow::util::ArrowLog::IsLevelEnabled(log_level)) { - arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); - } + arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); } } // namespace flight_sql From 044079b2496a19832a9d5d7fc4c992e6110205a9 Mon Sep 17 00:00:00 2001 From: "Alina (Xi) Li" Date: Fri, 26 Sep 2025 17:15:26 -0700 Subject: [PATCH 6/6] Register log if log level is not info The default log level is INFO --- .../arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc index 7481adc3f06..70e94def222 100644 --- a/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc +++ b/cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_driver.cc @@ -79,7 +79,10 @@ void FlightSqlDriver::RegisterLog() { // Enable driver logging. Log files are not supported on Windows yet, since GLOG is not // tested fully on Windows. - arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); + // Info log level is enabled by default. + if (log_level != ArrowLogLevel::ARROW_INFO) { + arrow::util::ArrowLog::StartArrowLog("arrow-flight-sql-odbc", log_level); + } } } // namespace flight_sql