From c8120b0b5aabb7a1c20cd3937532edfbe2b29789 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Fri, 14 May 2021 19:20:12 +0400 Subject: [PATCH 01/20] Bump submodules --- contrib/folly | 2 +- contrib/googletest | 2 +- contrib/nanodbc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/folly b/contrib/folly index 0938cf415..df9437f96 160000 --- a/contrib/folly +++ b/contrib/folly @@ -1 +1 @@ -Subproject commit 0938cf4150a97143125ac892303a90cef7ad2254 +Subproject commit df9437f96dd66a23e1f5f0efdca48b2266afc3bd diff --git a/contrib/googletest b/contrib/googletest index e3f0319d8..662fe38e4 160000 --- a/contrib/googletest +++ b/contrib/googletest @@ -1 +1 @@ -Subproject commit e3f0319d89f4cbf32993de595d984183b1a9fc57 +Subproject commit 662fe38e44900c007eccb65a5d2ea19df7bd520e diff --git a/contrib/nanodbc b/contrib/nanodbc index aaa3b89db..1a303f102 160000 --- a/contrib/nanodbc +++ b/contrib/nanodbc @@ -1 +1 @@ -Subproject commit aaa3b89db6e5852756a9b6fdfdce404af73adfb6 +Subproject commit 1a303f1028baf973d92bec037f92a2516d7060a9 From 21bf870d5eb968323c41ae1f65f69e0a85fb7cc9 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Fri, 14 May 2021 19:22:01 +0400 Subject: [PATCH 02/20] Disable nanodbc tests for AppleClang, until nanodbc compilation is fixed in upstream --- driver/test/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/driver/test/CMakeLists.txt b/driver/test/CMakeLists.txt index da332c8e9..04c00e6b2 100644 --- a/driver/test/CMakeLists.txt +++ b/driver/test/CMakeLists.txt @@ -110,6 +110,10 @@ function (declare_odbc_test_targets libname UNICODE) endforeach () if (TARGET nanodbc + + # Todo: enable once nanodbc compilation is fixed for recent AppleClang. + AND NOT ${CMAKE_CXX_COMPILER_ID} STREQUAL AppleClang + # AND ( # (NANODBC_ENABLE_UNICODE AND UNICODE) OR # (NOT NANODBC_ENABLE_UNICODE AND NOT UNICODE) From e8d25073765077712820d8b5c4d82f49e5a8900b Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Mon, 24 May 2021 17:47:12 +0400 Subject: [PATCH 03/20] Bump submodules --- contrib/folly | 2 +- contrib/googletest | 2 +- contrib/nanodbc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/folly b/contrib/folly index df9437f96..0fee9dfce 160000 --- a/contrib/folly +++ b/contrib/folly @@ -1 +1 @@ -Subproject commit df9437f96dd66a23e1f5f0efdca48b2266afc3bd +Subproject commit 0fee9dfce93e42b33ca8fb2c455b1d7cce8bebce diff --git a/contrib/googletest b/contrib/googletest index 662fe38e4..9741c4220 160000 --- a/contrib/googletest +++ b/contrib/googletest @@ -1 +1 @@ -Subproject commit 662fe38e44900c007eccb65a5d2ea19df7bd520e +Subproject commit 9741c42200b66abc708ee6da269a29c8bd912cee diff --git a/contrib/nanodbc b/contrib/nanodbc index 1a303f102..c0be39ab6 160000 --- a/contrib/nanodbc +++ b/contrib/nanodbc @@ -1 +1 @@ -Subproject commit 1a303f1028baf973d92bec037f92a2516d7060a9 +Subproject commit c0be39ab6983330a3aa8e8d4359421fceb7449ce From dee1ef80b23677cb87af704b7b4c5f4fdb164061 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Thu, 27 May 2021 17:16:34 +0400 Subject: [PATCH 04/20] Revert "Disable nanodbc tests for AppleClang, until nanodbc compilation is fixed in upstream" This reverts commit 21bf870d5eb968323c41ae1f65f69e0a85fb7cc9. --- driver/test/CMakeLists.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/driver/test/CMakeLists.txt b/driver/test/CMakeLists.txt index 04c00e6b2..da332c8e9 100644 --- a/driver/test/CMakeLists.txt +++ b/driver/test/CMakeLists.txt @@ -110,10 +110,6 @@ function (declare_odbc_test_targets libname UNICODE) endforeach () if (TARGET nanodbc - - # Todo: enable once nanodbc compilation is fixed for recent AppleClang. - AND NOT ${CMAKE_CXX_COMPILER_ID} STREQUAL AppleClang - # AND ( # (NANODBC_ENABLE_UNICODE AND UNICODE) OR # (NOT NANODBC_ENABLE_UNICODE AND NOT UNICODE) From 0c59a9747b0e9ec090536c94b4a2b318a6f1d16b Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Thu, 27 May 2021 17:45:30 +0400 Subject: [PATCH 05/20] Fix typo Proofread Add more info on installing Homebrew, Xcode, and Command Line Tools --- README.md | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 38afa619e..186cbcbec 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ The canonical repo for this driver is located at [https://github.com/ClickHouse/ See [LICENSE](LICENSE) file for licensing information. -## Table of content +## Table of contents - [Installation](#installation) - [Configuration](#configuration) @@ -53,7 +53,7 @@ If you don't see a package that matches your platforms, or the version of your s - [Building from sources](#building-from-sources) -Note, that it is always a good idea to install the driver from the corresponding **native** package (`.msi`, etc., which you can also easily create if you are building from sources), than use the binaries that were manually copied to some folder. +Note, that it is always a good idea to install the driver from the corresponding **native** package (if one is supported for your platform, like `.msi`, etc., which you can also easily create if you are building from sources), than use the binaries that were manually copied to a destination folder. Native packages will have all the dependency information so when you install the driver using a native package, all required run-time packages will be installed automatically. If you use manual packaging, i.e., just extract driver binaries to some folder, you also have to make sure that all the run-time dependencies are satisfied in your system manually: @@ -70,7 +70,7 @@ The next step is defining one or more DSNs, associated with the newly registered All this involves modifying a dedicated registry keys in case of MDAC, or editing `odbcinst.ini` (for driver registration) and `odbc.ini` (for DSN definition) files for UnixODBC or iODBC, directly or indirectly. -This will be done automatically using some default values if you are installing the driver using native installers. +This will be performed automatically using some default values if you are installing the driver using native installers. Otherwise, if you are configuring manually, or need to modify the default configuration created by the installer, please see the exact locations of files (or registry keys) that need to be modified in the corresponding section below: @@ -170,23 +170,25 @@ Configuration options above can be specified in the first `cmake` command (gener All modern Windows systems come with preinstalled MDAC driver manager. -Another run-time dependecies are `C++ Redistributable for Visual Studio 2017` or same for `2019`, etc., depending on the package being installed, however the required DLL's are redistributed with the `.msi` installer, and you can choose to install them from there, if you don't already have them installed in your system. +Another run-time dependecies are `C++ Redistributable for Visual Studio 2017` or same for `2019`, etc., depending on the package being installed, however the required DLL's are redistributed with the `.msi` installer, and you can choose to install them from there, if you don't have them installed in your system already. ### Run-time dependencies: macOS #### iODBC -Homebrew: execute the following in the terminal (assuming you have [Homebrew](https://brew.sh/) installed): +Execute the following in the terminal (assuming you have [Homebrew](https://brew.sh/) installed): ```sh +brew update brew install poco openssl icu4c libiodbc ``` #### UnixODBC -Homebrew: execute the following in the terminal (assuming you have [Homebrew](https://brew.sh/) installed): +Execute the following in the terminal (assuming you have [Homebrew](https://brew.sh/) installed): ```sh +brew update brew install poco openssl icu4c unixodbc ``` @@ -370,27 +372,46 @@ cmake --open . #### Build-time dependencies -You will need macOS 10.14 or later, Xcode 10 or later with Command Line Tools installed, as well as up-to-date [Homebrew](https://brew.sh/) available in the system. +You will need macOS 10.14 or later, Xcode 10 or later with Command Line Tools installed, as well as up-to-date Homebrew available in the system. + +Install [Homebrew](https://brew.sh/) using the following command, and follow the printed instructions on any additional steps required to complete the installation: + +```sh +/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" +``` + +Then, install the latest [Xcode](https://apps.apple.com/am/app/xcode/id497799835?mt=12) from App Store. Open it at least once to accept the end-user license agreement and automatically install the required components. + +Then, make sure that the latest Command Line Tools are installed and selected in the system: + +```sh +sudo rm -rf /Library/Developer/CommandLineTools +sudo xcode-select --install +``` + +Reboot. #### Build-time dependencies: iODBC -Homebrew: execute the following in the terminal (assuming you have [Homebrew](https://brew.sh/) installed): +Execute the following in the terminal: ```sh +brew update brew install git cmake make poco openssl icu4c libiodbc ``` #### Build-time dependencies: UnixODBC -Homebrew: execute the following in the terminal (assuming you have [Homebrew](https://brew.sh/) installed): +Execute the following in the terminal: ```sh +brew update brew install git cmake make poco openssl icu4c unixodbc ``` #### Build steps -Clone the repo with submodules: +Clone the repo recursively with submodules: ```sh git clone --recursive git@github.com:ClickHouse/clickhouse-odbc.git From c7ff1e5f3bb045b6b8d6fcef17b597e0b72b955c Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Thu, 27 May 2021 17:46:56 +0400 Subject: [PATCH 06/20] Bump submodules --- contrib/folly | 2 +- contrib/googletest | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/folly b/contrib/folly index 0fee9dfce..99fbca1df 160000 --- a/contrib/folly +++ b/contrib/folly @@ -1 +1 @@ -Subproject commit 0fee9dfce93e42b33ca8fb2c455b1d7cce8bebce +Subproject commit 99fbca1df19fdd21f1b831cad6f50ece94573675 diff --git a/contrib/googletest b/contrib/googletest index 9741c4220..a3460d1ae 160000 --- a/contrib/googletest +++ b/contrib/googletest @@ -1 +1 @@ -Subproject commit 9741c42200b66abc708ee6da269a29c8bd912cee +Subproject commit a3460d1aeeaa43fdf137a6adefef10ba0b59fe4b From 6341a974bb41071fa8ed735e337f9af136225fda Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Tue, 20 Jul 2021 17:35:09 +0400 Subject: [PATCH 07/20] Bump submodules --- contrib/folly | 2 +- contrib/googletest | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/folly b/contrib/folly index 99fbca1df..4548ec7bf 160000 --- a/contrib/folly +++ b/contrib/folly @@ -1 +1 @@ -Subproject commit 99fbca1df19fdd21f1b831cad6f50ece94573675 +Subproject commit 4548ec7bfd2753a1f4b63a605486a83bc72bc9c9 diff --git a/contrib/googletest b/contrib/googletest index a3460d1ae..8d51ffdfa 160000 --- a/contrib/googletest +++ b/contrib/googletest @@ -1 +1 @@ -Subproject commit a3460d1aeeaa43fdf137a6adefef10ba0b59fe4b +Subproject commit 8d51ffdfab10b3fba636ae69bc03da4b54f8c235 From b65ba52ecfd0291e31aaa6115955a2ad17360150 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Tue, 20 Jul 2021 23:51:11 +0400 Subject: [PATCH 08/20] Adjust ClientTestBase for different base classes Silence Disconnect logging --- driver/test/client_test_base.h | 14 ++++++++++---- driver/test/gtest_env.cpp | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/driver/test/client_test_base.h b/driver/test/client_test_base.h index 72d28554f..52b9a4683 100755 --- a/driver/test/client_test_base.h +++ b/driver/test/client_test_base.h @@ -6,11 +6,12 @@ #include -class ClientTestBase - : public ::testing::Test +template +class ClientTestBaseMixin + : public Base { public: - virtual ~ClientTestBase() { + virtual ~ClientTestBaseMixin() { if (hstmt) { SQLFreeHandle(SQL_HANDLE_STMT, hstmt); hstmt = nullptr; @@ -49,7 +50,7 @@ class ClientTestBase } if (hdbc) { - ODBC_CALL_ON_DBC_LOG(hdbc, SQLDisconnect(hdbc)); + /*ODBC_CALL_ON_DBC_LOG(hdbc, */SQLDisconnect(hdbc)/*)*/; ODBC_CALL_ON_DBC_THROW(hdbc, SQLFreeHandle(SQL_HANDLE_DBC, hdbc)); hdbc = nullptr; } @@ -65,3 +66,8 @@ class ClientTestBase SQLHDBC hdbc = nullptr; SQLHSTMT hstmt = nullptr; }; + +using ClientTestBase = ClientTestBaseMixin<::testing::Test>; + +template +using ClientTestWithParamBase = ClientTestBaseMixin<::testing::TestWithParam>; diff --git a/driver/test/gtest_env.cpp b/driver/test/gtest_env.cpp index 41572df07..e1e3eb866 100644 --- a/driver/test/gtest_env.cpp +++ b/driver/test/gtest_env.cpp @@ -25,7 +25,7 @@ TestEnvironment & TestEnvironment::getInstance() { return *environment_; } -const std::string& TestEnvironment::getDSN() { +const std::string & TestEnvironment::getDSN() { if (command_line_params_.size() != 1) throw std::runtime_error("Unable to extract positional command-line parameter value for DSN"); From 78400597b6ad947dd277dda673e17ff3100715ac Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Tue, 20 Jul 2021 23:52:42 +0400 Subject: [PATCH 09/20] Implement VerifyConnectionEarly functionality --- driver/config/config.cpp | 3 + driver/config/config.h | 1 + driver/config/ini_defines.h | 2 + driver/connection.cpp | 18 ++++++ driver/connection.h | 4 ++ driver/test/misc_it.cpp | 112 ++++++++++++++++++++++++++++++++++++ 6 files changed, 140 insertions(+) diff --git a/driver/config/config.cpp b/driver/config/config.cpp index 311f459e9..95c3c0f9b 100755 --- a/driver/config/config.cpp +++ b/driver/config/config.cpp @@ -51,6 +51,7 @@ void readDSNinfo(ConnInfo * ci, bool overwrite) { GET_CONFIG(server, INI_SERVER, INI_SERVER_DEFAULT); GET_CONFIG(port, INI_PORT, INI_PORT_DEFAULT); GET_CONFIG(timeout, INI_TIMEOUT, INI_TIMEOUT_DEFAULT); + GET_CONFIG(verify_connection_early, INI_VERIFYCONNECTIONEARLY, INI_VERIFYCONNECTIONEARLY_DEFAULT); GET_CONFIG(sslmode, INI_SSLMODE, INI_SSLMODE_DEFAULT); GET_CONFIG(database, INI_DATABASE, INI_DATABASE_DEFAULT); GET_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH, INI_STRINGMAXLENGTH_DEFAULT); @@ -90,6 +91,7 @@ void writeDSNinfo(const ConnInfo * ci) { WRITE_CONFIG(server, INI_SERVER); WRITE_CONFIG(port, INI_PORT); WRITE_CONFIG(timeout, INI_TIMEOUT); + WRITE_CONFIG(verify_connection_early, INI_VERIFYCONNECTIONEARLY); WRITE_CONFIG(sslmode, INI_SSLMODE); WRITE_CONFIG(database, INI_DATABASE); WRITE_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH); @@ -329,6 +331,7 @@ key_value_map_t readDSNInfo(const std::string & dsn_utf8) { INI_HOST, INI_PORT, INI_TIMEOUT, + INI_VERIFYCONNECTIONEARLY, INI_SSLMODE, INI_PRIVATEKEYFILE, INI_CERTIFICATEFILE, diff --git a/driver/config/config.h b/driver/config/config.h index 607332ac2..52011e04e 100755 --- a/driver/config/config.h +++ b/driver/config/config.h @@ -22,6 +22,7 @@ struct ConnInfo { std::string server; std::string port; std::string timeout; + std::string verify_connection_early; std::string sslmode; std::string privateKeyFile; std::string certificateFile; diff --git a/driver/config/ini_defines.h b/driver/config/ini_defines.h index 0c1e0c18e..cfc34e129 100755 --- a/driver/config/ini_defines.h +++ b/driver/config/ini_defines.h @@ -20,6 +20,7 @@ #define INI_HOST "Host" #define INI_PORT "Port" /* Port on which the ClickHouse is listening */ #define INI_TIMEOUT "Timeout" /* Connection timeout */ +#define INI_VERIFYCONNECTIONEARLY "VerifyConnectionEarly" #define INI_SSLMODE "SSLMode" /* Use 'require' for https connections */ #define INI_PRIVATEKEYFILE "PrivateKeyFile" #define INI_CERTIFICATEFILE "CertificateFile" @@ -43,6 +44,7 @@ #define INI_SERVER_DEFAULT "" #define INI_PORT_DEFAULT "" #define INI_TIMEOUT_DEFAULT "30" +#define INI_VERIFYCONNECTIONEARLY_DEFAULT "no" #define INI_SSLMODE_DEFAULT "" #define INI_DATABASE_DEFAULT "" #define INI_STRINGMAXLENGTH_DEFAULT "1048575" diff --git a/driver/connection.cpp b/driver/connection.cpp index 8f2c7184e..903239548 100644 --- a/driver/connection.cpp +++ b/driver/connection.cpp @@ -132,6 +132,10 @@ void Connection::connect(const std::string & connection_string) { session->setKeepAlive(true); session->setTimeout(Poco::Timespan(connection_timeout, 0), Poco::Timespan(timeout, 0), Poco::Timespan(timeout, 0)); session->setKeepAliveTimeout(Poco::Timespan(86400, 0)); + + if (verify_connection_early) { + verifyConnection(); + } } void Connection::resetConfiguration() { @@ -238,6 +242,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v timeout = typed_value; } } + else if (Poco::UTF8::icompare(key, INI_VERIFYCONNECTIONEARLY) == 0) { + recognized_key = true; + valid_value = (value.empty() || isYesOrNo(value)); + if (valid_value) { + verify_connection_early = isYes(value); + } + } else if (Poco::UTF8::icompare(key, INI_SSLMODE) == 0) { recognized_key = true; valid_value = ( @@ -445,6 +456,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v stringmaxlength = TypeInfo::string_max_size; } +void Connection::verifyConnection() { + LOG("Verifying connection and credentials..."); + auto & statement = allocateChild(); + statement.executeQuery("SELECT 1"); + statement.deallocateSelf(); +} + std::string Connection::buildCredentialsString() const { std::ostringstream user_password_base64; Poco::Base64Encoder base64_encoder(user_password_base64, Poco::BASE64_URL_ENCODING); diff --git a/driver/connection.h b/driver/connection.h index 94907e912..91126c761 100644 --- a/driver/connection.h +++ b/driver/connection.h @@ -29,6 +29,7 @@ class Connection std::uint16_t port = 0; std::uint32_t connection_timeout = 0; std::uint32_t timeout = 0; + bool verify_connection_early = false; std::string sslmode; std::string privateKeyFile; std::string certificateFile; @@ -84,6 +85,9 @@ class Connection // c) values deduced from values of other fields, if unintialized. void setConfiguration(const key_value_map_t & cs_fields, const key_value_map_t & dsn_fields); + // Verify the connection and credentials by trying to remotely execute a simple "SELECT 1" query. + void verifyConnection(); + private: std::unordered_map> descriptors; std::unordered_map> statements; diff --git a/driver/test/misc_it.cpp b/driver/test/misc_it.cpp index a2d4d026e..bf7f5e352 100755 --- a/driver/test/misc_it.cpp +++ b/driver/test/misc_it.cpp @@ -101,3 +101,115 @@ TEST_F(MiscellaneousTest, SQLGetData_ZeroOutputBufferSize) { ASSERT_EQ(SQLFetch(hstmt), SQL_NO_DATA); } + +enum class FailOn { + Connect, + Execute, + Never +}; + +// First parameter - parameter set name. +// Second parameter - extra name=value semicolon-separated string to append to the connection string. +// Third parameter - when to expect the failure, if any. +class ConnectionFailureReporing + : public ClientTestWithParamBase> +{ +protected: + virtual void SetUp() override { + using Base = ClientTestWithParamBase>; + Base::SetUp(); + + // As a precondition, check that by default the server is reachable, + // and we are able connect, authenticate, and execute queries successfully. + + const std::string query_orig = "SELECT 1"; + + const auto query = fromUTF8(query_orig); + auto * query_wptr = const_cast(query.c_str()); + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS)); + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecute(hstmt)); + + // Free the original Connection and Statement instances, and create a new Connection, + // but don't connect it yet - each test will do it on its own. + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLFreeHandle(SQL_HANDLE_STMT, hstmt)); + hstmt = nullptr; + + SQLDisconnect(hdbc); + ODBC_CALL_ON_DBC_THROW(hdbc, SQLFreeHandle(SQL_HANDLE_DBC, hdbc)); + + ODBC_CALL_ON_ENV_THROW(henv, SQLAllocHandle(SQL_HANDLE_DBC, henv, &hdbc)); + } +}; + +TEST_P(ConnectionFailureReporing, TryQuery) { + const auto & [/* unused */name, cs_extras, fail_on] = GetParam(); + + { + const auto & dsn_orig = TestEnvironment::getInstance().getDSN(); + std::string cs_orig = "DSN={" + dsn_orig + "};" + cs_extras; + const auto cs = fromUTF8(cs_orig); + auto * cs_wptr = const_cast(cs.c_str()); + + const auto rc = SQLDriverConnect(hdbc, NULL, cs_wptr, SQL_NTS, NULL, 0, NULL, SQL_DRIVER_NOPROMPT); + + if (fail_on == FailOn::Connect) { + ASSERT_EQ(rc, SQL_ERROR) << "Expected to fail on Connect!"; + return; + } + else { + ODBC_CALL_ON_DBC_THROW(hdbc, rc); + } + } + + ODBC_CALL_ON_DBC_THROW(hdbc, SQLAllocHandle(SQL_HANDLE_STMT, hdbc, &hstmt)); + + { + const std::string query_orig = "SELECT 1"; + + const auto query = fromUTF8(query_orig); + auto * query_wptr = const_cast(query.c_str()); + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS)); + + const auto rc = SQLExecute(hstmt); + + if (fail_on == FailOn::Execute) { + ASSERT_EQ(rc, SQL_ERROR) << "Expected to fail on Execute!"; + return; + } + else { + ODBC_CALL_ON_STMT_THROW(hstmt, rc); + } + } +} + +INSTANTIATE_TEST_SUITE_P( + MiscellaneousTest, + ConnectionFailureReporing, + ::testing::Values( + std::make_tuple("BadHost_FailOnConnect", "Host=some_bad_hostname;VerifyConnectionEarly=true", FailOn::Connect), + std::make_tuple("BadUsername_FailOnConnect", "UID=some_bad_username;VerifyConnectionEarly=true", FailOn::Connect), + std::make_tuple("BadPassword_FailOnConnect", "PWD=some_bad_password;VerifyConnectionEarly=true", FailOn::Connect), + + std::make_tuple("BadHost_FailOnExecute", "Host=some_bad_hostname;VerifyConnectionEarly=false", FailOn::Execute), + std::make_tuple("BadUsername_FailOnExecute", "UID=some_bad_username;VerifyConnectionEarly=false", FailOn::Execute), + std::make_tuple("BadPassword_FailOnExecute", "PWD=some_bad_password;VerifyConnectionEarly=false", FailOn::Execute), + + std::make_tuple("BadHost_FailOnExecuteByDefault", "Host=some_bad_hostname", FailOn::Execute), + std::make_tuple("BadUsername_FailOnExecuteByDefault", "UID=some_bad_username", FailOn::Execute), + std::make_tuple("BadPassword_FailOnExecuteByDefault", "PWD=some_bad_password", FailOn::Execute), + + std::make_tuple("BadHost_FailOnExecuteByDefault2", "Host=some_bad_hostname;VerifyConnectionEarly=", FailOn::Execute), + std::make_tuple("BadUsername_FailOnExecuteByDefault2", "UID=some_bad_username;VerifyConnectionEarly=", FailOn::Execute), + std::make_tuple("BadPassword_FailOnExecuteByDefault2", "PWD=some_bad_password;VerifyConnectionEarly=", FailOn::Execute), + + std::make_tuple("AllGood_VerifyConnectionEarly_Empty", "VerifyConnectionEarly=", FailOn::Never), + std::make_tuple("AllGood_VerifyConnectionEarly_True", "VerifyConnectionEarly=true", FailOn::Never), + std::make_tuple("AllGood_VerifyConnectionEarly_False", "VerifyConnectionEarly=false", FailOn::Never) + ), + [] (const auto & param_info) { + return std::get<0>(param_info.param); + } +); From a48fc58b98e38b2b6e71f014439ec49c3a513b17 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Wed, 21 Jul 2021 02:38:36 +0400 Subject: [PATCH 10/20] Add doc for VerifyConnectionEarly --- README.md | 35 ++++++++++++++++++----------------- driver/config/ini_defines.h | 2 +- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 186cbcbec..a4a74ce7c 100644 --- a/README.md +++ b/README.md @@ -80,23 +80,24 @@ Otherwise, if you are configuring manually, or need to modify the default config The list of DSN parameters recognized by the driver is as follows: -| Parameter | Default value | Description | -| :-----------------: | :----------------------------------------------------------------------------------------------------------------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `Url` | empty | URL that points to a running ClickHouse instance, may include username, password, port, database, etc. Also, see [URL query string](#url-query-string) | -| `Proto` | deduced from `Url`, or from `Port` and `SSLMode`: `https` if `443` or `8443` or `SSLMode` is not empty, `http` otherwise | Protocol, one of: `http`, `https` | -| `Server` or `Host` | deduced from `Url` | IP or hostname of a server with a running ClickHouse instance on it | -| `Port` | deduced from `Url`, or from `Proto`: `8443` if `https`, `8123` otherwise | Port on which the ClickHouse instance is listening | -| `Path` | `/query` | Path portion of the URL | -| `UID` or `Username` | `default` | User name | -| `PWD` or `Password` | empty | Password | -| `Database` | `default` | Database name to connect to | -| `Timeout` | `30` | Connection timeout | -| `SSLMode` | empty | Certificate verification method (used by TLS/SSL connections, ignored in Windows), one of: `allow`, `prefer`, `require`, use `allow` to enable [`SSL_VERIFY_PEER`](https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_verify.html) TLS/SSL certificate verification mode, [`SSL_VERIFY_PEER \| SSL_VERIFY_FAIL_IF_NO_PEER_CERT`](https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_verify.html) is used otherwise | -| `PrivateKeyFile` | empty | Path to private key file (used by TLS/SSL connections), can be empty if no private key file is used | -| `CertificateFile` | empty | Path to certificate file (used by TLS/SSL connections, ignored in Windows), if the private key and the certificate are stored in the same file, this can be empty if `PrivateKeyFile` is specified | -| `CALocation` | empty | Path to the file or directory containing the CA/root certificates (used by TLS/SSL connections, ignored in Windows) | -| `DriverLog` | `on` if `CMAKE_BUILD_TYPE` is `Debug`, `off` otherwise | Enable or disable the extended driver logging | -| `DriverLogFile` | `\temp\clickhouse-odbc-driver.log` on Windows, `/tmp/clickhouse-odbc-driver.log` otherwise | Path to the extended driver log file (used when `DriverLog` is `on`) | +| Parameter | Default value | Description | +| :---------------------: | :----------------------------------------------------------------------------------------------------------------------: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Url` | empty | URL that points to a running ClickHouse instance, may include username, password, port, database, etc. Also, see [URL query string](#url-query-string) | +| `Proto` | deduced from `Url`, or from `Port` and `SSLMode`: `https` if `443` or `8443` or `SSLMode` is not empty, `http` otherwise | Protocol, one of: `http`, `https` | +| `Server` or `Host` | deduced from `Url` | IP or hostname of a server with a running ClickHouse instance on it | +| `Port` | deduced from `Url`, or from `Proto`: `8443` if `https`, `8123` otherwise | Port on which the ClickHouse instance is listening | +| `Path` | `/query` | Path portion of the URL | +| `UID` or `Username` | `default` | User name | +| `PWD` or `Password` | empty | Password | +| `Database` | `default` | Database name to connect to | +| `Timeout` | `30` | Connection timeout | +| `VerifyConnectionEarly` | `off` | Verify the connection and credentials during `SQLConnect` and similar calls (adds a typical overhead of one trivial remote query execution), otherwise, possible connection-related failures will be detected later, during `SQLExecute` and similar calls | +| `SSLMode` | empty | Certificate verification method (used by TLS/SSL connections, ignored in Windows), one of: `allow`, `prefer`, `require`, use `allow` to enable [`SSL_VERIFY_PEER`](https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_verify.html) TLS/SSL certificate verification mode, [`SSL_VERIFY_PEER \| SSL_VERIFY_FAIL_IF_NO_PEER_CERT`](https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_verify.html) is used otherwise | +| `PrivateKeyFile` | empty | Path to private key file (used by TLS/SSL connections), can be empty if no private key file is used | +| `CertificateFile` | empty | Path to certificate file (used by TLS/SSL connections, ignored in Windows), if the private key and the certificate are stored in the same file, this can be empty if `PrivateKeyFile` is specified | +| `CALocation` | empty | Path to the file or directory containing the CA/root certificates (used by TLS/SSL connections, ignored in Windows) | +| `DriverLog` | `on` if `CMAKE_BUILD_TYPE` is `Debug`, `off` otherwise | Enable or disable the extended driver logging | +| `DriverLogFile` | `\temp\clickhouse-odbc-driver.log` on Windows, `/tmp/clickhouse-odbc-driver.log` otherwise | Path to the extended driver log file (used when `DriverLog` is `on`) | ### URL query string diff --git a/driver/config/ini_defines.h b/driver/config/ini_defines.h index cfc34e129..9526d64bc 100755 --- a/driver/config/ini_defines.h +++ b/driver/config/ini_defines.h @@ -44,7 +44,7 @@ #define INI_SERVER_DEFAULT "" #define INI_PORT_DEFAULT "" #define INI_TIMEOUT_DEFAULT "30" -#define INI_VERIFYCONNECTIONEARLY_DEFAULT "no" +#define INI_VERIFYCONNECTIONEARLY_DEFAULT "off" #define INI_SSLMODE_DEFAULT "" #define INI_DATABASE_DEFAULT "" #define INI_STRINGMAXLENGTH_DEFAULT "1048575" From 703b6663a367e7f0a828e8b41b657f68ef5d8fd0 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Wed, 21 Jul 2021 18:32:48 +0400 Subject: [PATCH 11/20] Implement HugeIntAsString functionality --- README.md | 1 + driver/api/impl/impl.cpp | 3 +- driver/api/odbc.cpp | 14 +++--- driver/config/config.cpp | 8 +-- driver/config/config.h | 1 + driver/config/ini_defines.h | 6 ++- driver/connection.cpp | 23 ++++++++- driver/connection.h | 4 ++ driver/environment.h | 2 +- driver/statement.cpp | 2 +- driver/statement.h | 2 +- driver/test/client_test_base.h | 16 ++++-- driver/test/misc_it.cpp | 92 ++++++++++++++++++++++++++++++---- 13 files changed, 143 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index a4a74ce7c..20fbb0733 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,7 @@ The list of DSN parameters recognized by the driver is as follows: | `PrivateKeyFile` | empty | Path to private key file (used by TLS/SSL connections), can be empty if no private key file is used | | `CertificateFile` | empty | Path to certificate file (used by TLS/SSL connections, ignored in Windows), if the private key and the certificate are stored in the same file, this can be empty if `PrivateKeyFile` is specified | | `CALocation` | empty | Path to the file or directory containing the CA/root certificates (used by TLS/SSL connections, ignored in Windows) | +| `HugeIntAsString` | `off` | Report integer column types that may underflow or overflow 64-bit signed integer (`SQL_BIGINT`) as a `String`/`SQL_VARCHAR` | | `DriverLog` | `on` if `CMAKE_BUILD_TYPE` is `Debug`, `off` otherwise | Enable or disable the extended driver logging | | `DriverLogFile` | `\temp\clickhouse-odbc-driver.log` on Windows, `/tmp/clickhouse-odbc-driver.log` otherwise | Path to the extended driver log file (used when `DriverLog` is `on`) | diff --git a/driver/api/impl/impl.cpp b/driver/api/impl/impl.cpp index 5b3d5405e..8d90e6934 100644 --- a/driver/api/impl/impl.cpp +++ b/driver/api/impl/impl.cpp @@ -1269,7 +1269,8 @@ SQLRETURN fillBinding( } if (binding_info.c_type == SQL_C_DEFAULT) { - binding_info.c_type = convertSQLTypeToCType(statement.getTypeInfo(result_set.getColumnInfo(column_idx).type_without_parameters).sql_type); + const auto & column_info = result_set.getColumnInfo(column_idx); + binding_info.c_type = convertSQLTypeToCType(statement.getTypeInfo(column_info.type, column_info.type_without_parameters).sql_type); } if ( diff --git a/driver/api/odbc.cpp b/driver/api/odbc.cpp index df6fa9b5d..3b7d6c72f 100755 --- a/driver/api/odbc.cpp +++ b/driver/api/odbc.cpp @@ -968,8 +968,8 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)( : public ResultMutator { public: - explicit ColumnsMutator(Environment & env_) - : env(env_) + explicit ColumnsMutator(Statement & statement_) + : statement(statement_) { } @@ -999,7 +999,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)( tmp_column_info.updateTypeInfo(); }, row.fields.at(5).data); - const TypeInfo & type_info = env.getTypeInfo(tmp_column_info.type, tmp_column_info.type_without_parameters); + const TypeInfo & type_info = statement.getTypeInfo(tmp_column_info.type, tmp_column_info.type_without_parameters); row.fields.at(4).data = DataSourceType{type_info.sql_type}; row.fields.at(5).data = DataSourceType{type_info.sql_type_name}; @@ -1009,7 +1009,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)( } private: - Environment & env; + Statement & statement; }; auto func = [&](Statement & statement) { @@ -1093,7 +1093,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)( } query << " ORDER BY TABLE_CAT, TABLE_SCHEM, TABLE_NAME, ORDINAL_POSITION"; - statement.executeQuery(query.str(), std::make_unique(statement.getParent().getParent())); + statement.executeQuery(query.str(), std::make_unique(statement)); return SQL_SUCCESS; }; @@ -1171,13 +1171,13 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLGetTypeInfo)( // are SQL_TYPE_DATE, SQL_TYPE_TIME, and SQL_TYPE_TIMESTAMP, respectively; // in ODBC 2.x, the data types are SQL_DATE, SQL_TIME, and SQL_TIMESTAMP. { - auto info = statement.getParent().getParent().getTypeInfo("Date"); + auto info = statement.getTypeInfo("Date", "Date"); info.sql_type = SQL_DATE; add_query_for_type("Date", info); } { - auto info = statement.getParent().getParent().getTypeInfo("DateTime"); + auto info = statement.getTypeInfo("DateTime", "DateTime"); info.sql_type = SQL_TIMESTAMP; add_query_for_type("DateTime", info); } diff --git a/driver/config/config.cpp b/driver/config/config.cpp index 95c3c0f9b..59fac5d3b 100755 --- a/driver/config/config.cpp +++ b/driver/config/config.cpp @@ -51,9 +51,10 @@ void readDSNinfo(ConnInfo * ci, bool overwrite) { GET_CONFIG(server, INI_SERVER, INI_SERVER_DEFAULT); GET_CONFIG(port, INI_PORT, INI_PORT_DEFAULT); GET_CONFIG(timeout, INI_TIMEOUT, INI_TIMEOUT_DEFAULT); - GET_CONFIG(verify_connection_early, INI_VERIFYCONNECTIONEARLY, INI_VERIFYCONNECTIONEARLY_DEFAULT); + GET_CONFIG(verify_connection_early, INI_VERIFY_CONNECTION_EARLY, INI_VERIFY_CONNECTION_EARLY_DEFAULT); GET_CONFIG(sslmode, INI_SSLMODE, INI_SSLMODE_DEFAULT); GET_CONFIG(database, INI_DATABASE, INI_DATABASE_DEFAULT); + GET_CONFIG(huge_int_as_string, INI_HUGE_INT_AS_STRING, INI_HUGE_INT_AS_STRING_DEFAULT); GET_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH, INI_STRINGMAXLENGTH_DEFAULT); GET_CONFIG(driverlog, INI_DRIVERLOG, INI_DRIVERLOG_DEFAULT); GET_CONFIG(driverlogfile, INI_DRIVERLOGFILE, INI_DRIVERLOGFILE_DEFAULT); @@ -91,9 +92,10 @@ void writeDSNinfo(const ConnInfo * ci) { WRITE_CONFIG(server, INI_SERVER); WRITE_CONFIG(port, INI_PORT); WRITE_CONFIG(timeout, INI_TIMEOUT); - WRITE_CONFIG(verify_connection_early, INI_VERIFYCONNECTIONEARLY); + WRITE_CONFIG(verify_connection_early, INI_VERIFY_CONNECTION_EARLY); WRITE_CONFIG(sslmode, INI_SSLMODE); WRITE_CONFIG(database, INI_DATABASE); + WRITE_CONFIG(huge_int_as_string, INI_HUGE_INT_AS_STRING); WRITE_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH); WRITE_CONFIG(driverlog, INI_DRIVERLOG); WRITE_CONFIG(driverlogfile, INI_DRIVERLOGFILE); @@ -331,7 +333,7 @@ key_value_map_t readDSNInfo(const std::string & dsn_utf8) { INI_HOST, INI_PORT, INI_TIMEOUT, - INI_VERIFYCONNECTIONEARLY, + INI_VERIFY_CONNECTION_EARLY, INI_SSLMODE, INI_PRIVATEKEYFILE, INI_CERTIFICATEFILE, diff --git a/driver/config/config.h b/driver/config/config.h index 52011e04e..4b9922697 100755 --- a/driver/config/config.h +++ b/driver/config/config.h @@ -28,6 +28,7 @@ struct ConnInfo { std::string certificateFile; std::string caLocation; std::string database; + std::string huge_int_as_string; std::string stringmaxlength; std::string driverlog; std::string driverlogfile; diff --git a/driver/config/ini_defines.h b/driver/config/ini_defines.h index 9526d64bc..cd542cc29 100755 --- a/driver/config/ini_defines.h +++ b/driver/config/ini_defines.h @@ -20,13 +20,14 @@ #define INI_HOST "Host" #define INI_PORT "Port" /* Port on which the ClickHouse is listening */ #define INI_TIMEOUT "Timeout" /* Connection timeout */ -#define INI_VERIFYCONNECTIONEARLY "VerifyConnectionEarly" +#define INI_VERIFY_CONNECTION_EARLY "VerifyConnectionEarly" #define INI_SSLMODE "SSLMode" /* Use 'require' for https connections */ #define INI_PRIVATEKEYFILE "PrivateKeyFile" #define INI_CERTIFICATEFILE "CertificateFile" #define INI_CALOCATION "CALocation" #define INI_PATH "Path" /* Path portion of the URL */ #define INI_DATABASE "Database" /* Database Name */ +#define INI_HUGE_INT_AS_STRING "HugeIntAsString" #define INI_STRINGMAXLENGTH "StringMaxLength" #define INI_DRIVERLOG "DriverLog" #define INI_DRIVERLOGFILE "DriverLogFile" @@ -44,9 +45,10 @@ #define INI_SERVER_DEFAULT "" #define INI_PORT_DEFAULT "" #define INI_TIMEOUT_DEFAULT "30" -#define INI_VERIFYCONNECTIONEARLY_DEFAULT "off" +#define INI_VERIFY_CONNECTION_EARLY_DEFAULT "off" #define INI_SSLMODE_DEFAULT "" #define INI_DATABASE_DEFAULT "" +#define INI_HUGE_INT_AS_STRING_DEFAULT "off" #define INI_STRINGMAXLENGTH_DEFAULT "1048575" #ifdef NDEBUG diff --git a/driver/connection.cpp b/driver/connection.cpp index 903239548..bb3ae887f 100644 --- a/driver/connection.cpp +++ b/driver/connection.cpp @@ -53,6 +53,20 @@ Connection::Connection(Environment & environment) resetConfiguration(); } +const TypeInfo & Connection::getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const { + auto tmp_type_name = type_name; + auto tmp_type_name_without_parameters = type_name_without_parameters; + + const auto tmp_type_without_parameters_id = convertUnparametrizedTypeNameToTypeId(tmp_type_name_without_parameters); + + if (huge_int_as_string && tmp_type_without_parameters_id == DataSourceTypeId::UInt64) { + tmp_type_name = "String"; + tmp_type_name_without_parameters = "String"; + } + + return getParent().getTypeInfo(tmp_type_name, tmp_type_name_without_parameters); +} + void Connection::connect(const std::string & connection_string) { if (session && session->connected()) throw SqlException("Connection name in use", "08002"); @@ -242,7 +256,7 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v timeout = typed_value; } } - else if (Poco::UTF8::icompare(key, INI_VERIFYCONNECTIONEARLY) == 0) { + else if (Poco::UTF8::icompare(key, INI_VERIFY_CONNECTION_EARLY) == 0) { recognized_key = true; valid_value = (value.empty() || isYesOrNo(value)); if (valid_value) { @@ -296,6 +310,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v database = value; } } + else if (Poco::UTF8::icompare(key, INI_HUGE_INT_AS_STRING) == 0) { + recognized_key = true; + valid_value = (value.empty() || isYesOrNo(value)); + if (valid_value) { + huge_int_as_string = isYes(value); + } + } else if (Poco::UTF8::icompare(key, INI_STRINGMAXLENGTH) == 0) { recognized_key = true; unsigned int typed_value = 0; diff --git a/driver/connection.h b/driver/connection.h index 91126c761..646689364 100644 --- a/driver/connection.h +++ b/driver/connection.h @@ -37,6 +37,7 @@ class Connection std::string path; std::string default_format; std::string database; + bool huge_int_as_string = false; std::int32_t stringmaxlength = 0; public: @@ -49,6 +50,9 @@ class Connection public: explicit Connection(Environment & environment); + // Lookup TypeInfo for given name of type. + const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const; + void connect(const std::string & connection_string); // Return a Base64 encoded string of "user:password". diff --git a/driver/environment.h b/driver/environment.h index 663dc1ab3..3d6868d4e 100644 --- a/driver/environment.h +++ b/driver/environment.h @@ -20,7 +20,7 @@ class Environment template T & allocateChild(); template void deallocateChild(SQLHANDLE) noexcept; - const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters = "") const; + const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const; public: #if defined(SQL_OV_ODBC3_80) diff --git a/driver/statement.cpp b/driver/statement.cpp index 43c088766..621fd532c 100755 --- a/driver/statement.cpp +++ b/driver/statement.cpp @@ -25,7 +25,7 @@ Statement::~Statement() { } const TypeInfo & Statement::getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const { - return getParent().getParent().getTypeInfo(type_name, type_name_without_parameters); + return getParent().getTypeInfo(type_name, type_name_without_parameters); } void Statement::prepareQuery(const std::string & q) { diff --git a/driver/statement.h b/driver/statement.h index 8f41797ec..a8eb9398f 100644 --- a/driver/statement.h +++ b/driver/statement.h @@ -23,7 +23,7 @@ class Statement virtual ~Statement(); /// Lookup TypeInfo for given name of type. - const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters = "") const; + const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const; bool isPrepared() const; diff --git a/driver/test/client_test_base.h b/driver/test/client_test_base.h index 52b9a4683..dd0b8d944 100755 --- a/driver/test/client_test_base.h +++ b/driver/test/client_test_base.h @@ -11,6 +11,11 @@ class ClientTestBaseMixin : public Base { public: + ClientTestBaseMixin(bool skip_connect = false) + : skip_connect_(skip_connect) + { + } + virtual ~ClientTestBaseMixin() { if (hstmt) { SQLFreeHandle(SQL_HANDLE_STMT, hstmt); @@ -36,11 +41,13 @@ class ClientTestBaseMixin ODBC_CALL_ON_ENV_THROW(henv, SQLAllocHandle(SQL_HANDLE_DBC, henv, &hdbc)); - const auto dsn = fromUTF8(TestEnvironment::getInstance().getDSN()); - auto * dsn_wptr = const_cast(dsn.c_str()); + if (!skip_connect_) { + const auto dsn = fromUTF8(TestEnvironment::getInstance().getDSN()); + auto * dsn_wptr = const_cast(dsn.c_str()); - ODBC_CALL_ON_DBC_THROW(hdbc, SQLConnect(hdbc, dsn_wptr, SQL_NTS, NULL, 0, NULL, 0)); - ODBC_CALL_ON_DBC_THROW(hdbc, SQLAllocHandle(SQL_HANDLE_STMT, hdbc, &hstmt)); + ODBC_CALL_ON_DBC_THROW(hdbc, SQLConnect(hdbc, dsn_wptr, SQL_NTS, NULL, 0, NULL, 0)); + ODBC_CALL_ON_DBC_THROW(hdbc, SQLAllocHandle(SQL_HANDLE_STMT, hdbc, &hstmt)); + } } virtual void TearDown() override { @@ -62,6 +69,7 @@ class ClientTestBaseMixin } protected: + const bool skip_connect_; SQLHENV henv = nullptr; SQLHDBC hdbc = nullptr; SQLHSTMT hstmt = nullptr; diff --git a/driver/test/misc_it.cpp b/driver/test/misc_it.cpp index bf7f5e352..034a003e2 100755 --- a/driver/test/misc_it.cpp +++ b/driver/test/misc_it.cpp @@ -114,9 +114,11 @@ enum class FailOn { class ConnectionFailureReporing : public ClientTestWithParamBase> { +private: + using Base = ClientTestWithParamBase>; + protected: virtual void SetUp() override { - using Base = ClientTestWithParamBase>; Base::SetUp(); // As a precondition, check that by default the server is reachable, @@ -189,13 +191,13 @@ INSTANTIATE_TEST_SUITE_P( MiscellaneousTest, ConnectionFailureReporing, ::testing::Values( - std::make_tuple("BadHost_FailOnConnect", "Host=some_bad_hostname;VerifyConnectionEarly=true", FailOn::Connect), - std::make_tuple("BadUsername_FailOnConnect", "UID=some_bad_username;VerifyConnectionEarly=true", FailOn::Connect), - std::make_tuple("BadPassword_FailOnConnect", "PWD=some_bad_password;VerifyConnectionEarly=true", FailOn::Connect), + std::make_tuple("BadHost_FailOnConnect", "Host=some_bad_hostname;VerifyConnectionEarly=on", FailOn::Connect), + std::make_tuple("BadUsername_FailOnConnect", "UID=some_bad_username;VerifyConnectionEarly=on", FailOn::Connect), + std::make_tuple("BadPassword_FailOnConnect", "PWD=some_bad_password;VerifyConnectionEarly=on", FailOn::Connect), - std::make_tuple("BadHost_FailOnExecute", "Host=some_bad_hostname;VerifyConnectionEarly=false", FailOn::Execute), - std::make_tuple("BadUsername_FailOnExecute", "UID=some_bad_username;VerifyConnectionEarly=false", FailOn::Execute), - std::make_tuple("BadPassword_FailOnExecute", "PWD=some_bad_password;VerifyConnectionEarly=false", FailOn::Execute), + std::make_tuple("BadHost_FailOnExecute", "Host=some_bad_hostname;VerifyConnectionEarly=off", FailOn::Execute), + std::make_tuple("BadUsername_FailOnExecute", "UID=some_bad_username;VerifyConnectionEarly=off", FailOn::Execute), + std::make_tuple("BadPassword_FailOnExecute", "PWD=some_bad_password;VerifyConnectionEarly=off", FailOn::Execute), std::make_tuple("BadHost_FailOnExecuteByDefault", "Host=some_bad_hostname", FailOn::Execute), std::make_tuple("BadUsername_FailOnExecuteByDefault", "UID=some_bad_username", FailOn::Execute), @@ -205,11 +207,81 @@ INSTANTIATE_TEST_SUITE_P( std::make_tuple("BadUsername_FailOnExecuteByDefault2", "UID=some_bad_username;VerifyConnectionEarly=", FailOn::Execute), std::make_tuple("BadPassword_FailOnExecuteByDefault2", "PWD=some_bad_password;VerifyConnectionEarly=", FailOn::Execute), - std::make_tuple("AllGood_VerifyConnectionEarly_Empty", "VerifyConnectionEarly=", FailOn::Never), - std::make_tuple("AllGood_VerifyConnectionEarly_True", "VerifyConnectionEarly=true", FailOn::Never), - std::make_tuple("AllGood_VerifyConnectionEarly_False", "VerifyConnectionEarly=false", FailOn::Never) + std::make_tuple("AllGood_VerifyConnectionEarly_Empty", "VerifyConnectionEarly=", FailOn::Never), + std::make_tuple("AllGood_VerifyConnectionEarly_On", "VerifyConnectionEarly=on", FailOn::Never), + std::make_tuple("AllGood_VerifyConnectionEarly_Off", "VerifyConnectionEarly=off", FailOn::Never) ), [] (const auto & param_info) { return std::get<0>(param_info.param); } ); + +// First parameter - original "huge" integer type. +// Second parameter - a tuple of: +// First parameter - parameter set name. +// Second parameter - extra name=value semicolon-separated string to append to the connection string. +// Third parameter - expected reported column type. +class HugeIntTypeReporting + : public ClientTestWithParamBase>> +{ +private: + using Base = ClientTestWithParamBase>>; + +public: + HugeIntTypeReporting() + : Base(/*skip_connect = */true) + { + } + + void connect(const std::string & connection_string) { + ASSERT_EQ(hstmt, nullptr); + + const auto cs = fromUTF8(connection_string); + auto * cs_wptr = const_cast(cs.c_str()); + + ODBC_CALL_ON_DBC_THROW(hdbc, SQLDriverConnect(hdbc, NULL, cs_wptr, SQL_NTS, NULL, 0, NULL, SQL_DRIVER_NOPROMPT)); + ODBC_CALL_ON_DBC_THROW(hdbc, SQLAllocHandle(SQL_HANDLE_STMT, hdbc, &hstmt)); + } +}; + +TEST_P(HugeIntTypeReporting, Check) { + const auto & [type, params] = GetParam(); + const auto & [/* unused */name, cs_extras, expected_sql_type] = params; + + const auto & dsn = TestEnvironment::getInstance().getDSN(); + const auto cs = "DSN={" + dsn + "};" + cs_extras; + connect(cs); + + const auto query_orig = "SELECT CAST('0', '" + type + "') AS col"; + + const auto query = fromUTF8(query_orig); + auto * query_wptr = const_cast(query.c_str()); + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS)); + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecute(hstmt)); + + SQLLEN sql_type = SQL_TYPE_NULL; + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLColAttribute(hstmt, 1, SQL_DESC_TYPE, NULL, 0, NULL, &sql_type)); + + ASSERT_EQ(sql_type, expected_sql_type); +} + +INSTANTIATE_TEST_SUITE_P( + MiscellaneousTest, + HugeIntTypeReporting, + ::testing::Combine( + ::testing::Values( + "UInt64"/*, "Int128", "UInt128", "Int256", "UInt256"*/ + ), + ::testing::Values( + std::make_tuple("HugeIntAsString_Default", "", SQL_BIGINT), + std::make_tuple("HugeIntAsString_Empty", "HugeIntAsString=", SQL_BIGINT), + std::make_tuple("HugeIntAsString_On", "HugeIntAsString=on", SQL_VARCHAR), + std::make_tuple("HugeIntAsString_Off", "HugeIntAsString=off", SQL_BIGINT) + ) + ), + [] (const auto & param_info) { + return std::get<0>(std::get<1>(param_info.param)) + "_with_" + std::get<0>(param_info.param); + } +); From 4ab314c7272b444436a55a9a3639144e9a0c7074 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Wed, 21 Jul 2021 19:15:33 +0400 Subject: [PATCH 12/20] Add handling for Nothing type --- driver/test/misc_it.cpp | 26 ++++++++++++++++++++------ driver/utils/type_info.cpp | 7 +++++++ driver/utils/type_info.h | 2 +- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/driver/test/misc_it.cpp b/driver/test/misc_it.cpp index 034a003e2..5d2d54ba9 100755 --- a/driver/test/misc_it.cpp +++ b/driver/test/misc_it.cpp @@ -64,8 +64,7 @@ TEST_F(MiscellaneousTest, SQLGetData_ZeroOutputBufferSize) { const auto query = fromUTF8(query_orig); auto * query_wptr = const_cast(query.c_str()); - ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS)); - ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecute(hstmt)); + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecDirect(hstmt, query_wptr, SQL_NTS)); SQLTCHAR col[100] = {}; SQLLEN col_ind = 0; @@ -102,6 +101,23 @@ TEST_F(MiscellaneousTest, SQLGetData_ZeroOutputBufferSize) { ASSERT_EQ(SQLFetch(hstmt), SQL_NO_DATA); } +TEST_F(MiscellaneousTest, NullableNothing) { + const std::string query_orig = "SELECT NULL as col"; + + const auto query = fromUTF8(query_orig); + auto * query_wptr = const_cast(query.c_str()); + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecDirect(hstmt, query_wptr, SQL_NTS)); + + SQLSMALLINT sql_type = SQL_BIT; + SQLSMALLINT nullable = SQL_NULLABLE_UNKNOWN; + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLDescribeCol(hstmt, 1, NULL, 0, NULL, &sql_type, NULL, NULL, &nullable)); + + EXPECT_EQ(sql_type, SQL_TYPE_NULL); + EXPECT_EQ(nullable, SQL_NULLABLE); +} + enum class FailOn { Connect, Execute, @@ -129,8 +145,7 @@ class ConnectionFailureReporing const auto query = fromUTF8(query_orig); auto * query_wptr = const_cast(query.c_str()); - ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS)); - ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecute(hstmt)); + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecDirect(hstmt, query_wptr, SQL_NTS)); // Free the original Connection and Statement instances, and create a new Connection, // but don't connect it yet - each test will do it on its own. @@ -257,8 +272,7 @@ TEST_P(HugeIntTypeReporting, Check) { const auto query = fromUTF8(query_orig); auto * query_wptr = const_cast(query.c_str()); - ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS)); - ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecute(hstmt)); + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecDirect(hstmt, query_wptr, SQL_NTS)); SQLLEN sql_type = SQL_TYPE_NULL; diff --git a/driver/utils/type_info.cpp b/driver/utils/type_info.cpp index 2ffdd0241..1f5545013 100644 --- a/driver/utils/type_info.cpp +++ b/driver/utils/type_info.cpp @@ -33,6 +33,8 @@ const std::map types_g = { TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, // todo: remove {"LowCardinality(FixedString)", TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, // todo: remove + + {"Nothing", TypeInfo {"NULL", true, SQL_TYPE_NULL, 1, 1}}, }; DataSourceTypeId convertUnparametrizedTypeNameToTypeId(const std::string & type_name) { @@ -101,6 +103,7 @@ std::string convertTypeIdToUnparametrizedCanonicalTypeName(DataSourceTypeId type SQLSMALLINT convertSQLTypeToCType(SQLSMALLINT sql_type) noexcept { switch (sql_type) { + case SQL_TYPE_NULL: case SQL_CHAR: case SQL_VARCHAR: case SQL_LONGVARCHAR: @@ -447,6 +450,10 @@ std::string convertSQLTypeToDataSourceType(const BoundTypeInfo & type_info) { std::string type_name; switch (type_info.sql_type) { + case SQL_TYPE_NULL: + type_name = set_nullability("Nothing"); + break; + case SQL_WCHAR: case SQL_CHAR: type_name = set_nullability("String"); diff --git a/driver/utils/type_info.h b/driver/utils/type_info.h index 2df94bc27..d895bd4fc 100755 --- a/driver/utils/type_info.h +++ b/driver/utils/type_info.h @@ -21,7 +21,7 @@ struct TypeInfo { SQLSMALLINT sql_type; // https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size-decimal-digits-transfer-octet-length-and-display-size - int32_t column_size; // max width of value in textual represntation, e.g. number of decimal digits fror numeric types. + int32_t column_size; // max width of value in textual represntation, e.g. number of decimal digits for numeric types. int32_t octet_length; // max binary size of value in memory. static constexpr auto string_max_size = 0xFFFFFF; From 4bfdda72ee5a1b458e6c768cd6ec3515e066c444 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Thu, 22 Jul 2021 04:46:00 +0400 Subject: [PATCH 13/20] Add DateTime64 support Some refactoring --- driver/api/odbc.cpp | 3 +- driver/format/ODBCDriver2.cpp | 15 ++- driver/format/ODBCDriver2.h | 5 +- driver/format/RowBinaryWithNamesAndTypes.cpp | 33 ++++-- driver/format/RowBinaryWithNamesAndTypes.h | 16 ++- driver/result_set.cpp | 38 +++++-- driver/result_set.h | 12 ++- driver/statement.cpp | 8 +- driver/utils/type_info.cpp | 20 ++-- driver/utils/type_info.h | 102 +++++++++++++++++-- driver/utils/type_parser.cpp | 13 +++ 11 files changed, 206 insertions(+), 59 deletions(-) diff --git a/driver/api/odbc.cpp b/driver/api/odbc.cpp index 3b7d6c72f..dca8604ba 100755 --- a/driver/api/odbc.cpp +++ b/driver/api/odbc.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -984,7 +985,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)( TypeAst ast; if (parser.parse(&ast)) { - tmp_column_info.assignTypeInfo(ast); + tmp_column_info.assignTypeInfo(ast, Poco::Timezone::name()); if (convertUnparametrizedTypeNameToTypeId(tmp_column_info.type_without_parameters) == DataSourceTypeId::Unknown) { // Interpret all unknown types as String. diff --git a/driver/format/ODBCDriver2.cpp b/driver/format/ODBCDriver2.cpp index 4bbd52b26..2eb843fb6 100644 --- a/driver/format/ODBCDriver2.cpp +++ b/driver/format/ODBCDriver2.cpp @@ -1,7 +1,7 @@ #include "driver/format/ODBCDriver2.h" #include "driver/utils/resize_without_initialization.h" -ODBCDriver2ResultSet::ODBCDriver2ResultSet(AmortizedIStreamReader & stream, std::unique_ptr && mutator) +ODBCDriver2ResultSet::ODBCDriver2ResultSet(const std::string & timezone, AmortizedIStreamReader & stream, std::unique_ptr && mutator) : ResultSet(stream, std::move(mutator)) { std::int32_t num_header_rows = 0; @@ -36,7 +36,7 @@ ODBCDriver2ResultSet::ODBCDriver2ResultSet(AmortizedIStreamReader & stream, std: TypeAst ast; if (parser.parse(&ast)) { - columns_info[i].assignTypeInfo(ast); + columns_info[i].assignTypeInfo(ast, timezone); if (convertUnparametrizedTypeNameToTypeId(columns_info[i].type_without_parameters) == DataSourceTypeId::Unknown) { // Interpret all unknown types as String. @@ -131,6 +131,7 @@ void ODBCDriver2ResultSet::readValue(Field & dest, ColumnInfo & column_info) { else switch (column_info.type_without_parameters_id) { case DataSourceTypeId::Date: readValueAs>(value, dest, column_info); break; case DataSourceTypeId::DateTime: readValueAs>(value, dest, column_info); break; + case DataSourceTypeId::DateTime64: readValueAs>(value, dest, column_info); break; case DataSourceTypeId::Decimal: readValueAs>(value, dest, column_info); break; case DataSourceTypeId::Decimal32: readValueAs>(value, dest, column_info); break; case DataSourceTypeId::Decimal64: readValueAs>(value, dest, column_info); break; @@ -168,6 +169,10 @@ void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType::template to_value>::convert(src, dest); } +void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType & dest, ColumnInfo & column_info) { + return value_manip::from_value::template to_value>::convert(src, dest); +} + void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType & dest, ColumnInfo & column_info) { return value_manip::from_value::template to_value>::convert(src, dest); } @@ -240,13 +245,13 @@ void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType::template to_value>::convert(src, dest); } -ODBCDriver2ResultReader::ODBCDriver2ResultReader(std::istream & raw_stream, std::unique_ptr && mutator) - : ResultReader(raw_stream, std::move(mutator)) +ODBCDriver2ResultReader::ODBCDriver2ResultReader(const std::string & timezone_, std::istream & raw_stream, std::unique_ptr && mutator) + : ResultReader(timezone_, raw_stream, std::move(mutator)) { if (stream.eof()) return; - result_set = std::make_unique(stream, releaseMutator()); + result_set = std::make_unique(timezone, stream, releaseMutator()); } bool ODBCDriver2ResultReader::advanceToNextResultSet() { diff --git a/driver/format/ODBCDriver2.h b/driver/format/ODBCDriver2.h index 4ce658af9..6e4fd9502 100755 --- a/driver/format/ODBCDriver2.h +++ b/driver/format/ODBCDriver2.h @@ -8,7 +8,7 @@ class ODBCDriver2ResultSet : public ResultSet { public: - explicit ODBCDriver2ResultSet(AmortizedIStreamReader & stream, std::unique_ptr && mutator); + explicit ODBCDriver2ResultSet(const std::string & timezone, AmortizedIStreamReader & stream, std::unique_ptr && mutator); virtual ~ODBCDriver2ResultSet() override = default; protected: @@ -32,6 +32,7 @@ class ODBCDriver2ResultSet void readValue(std::string & src, DataSourceType< DataSourceTypeId::Date > & dest, ColumnInfo & column_info); void readValue(std::string & src, DataSourceType< DataSourceTypeId::DateTime > & dest, ColumnInfo & column_info); + void readValue(std::string & src, DataSourceType< DataSourceTypeId::DateTime64 > & dest, ColumnInfo & column_info); void readValue(std::string & src, DataSourceType< DataSourceTypeId::Decimal > & dest, ColumnInfo & column_info); void readValue(std::string & src, DataSourceType< DataSourceTypeId::Decimal32 > & dest, ColumnInfo & column_info); void readValue(std::string & src, DataSourceType< DataSourceTypeId::Decimal64 > & dest, ColumnInfo & column_info); @@ -61,7 +62,7 @@ class ODBCDriver2ResultReader : public ResultReader { public: - explicit ODBCDriver2ResultReader(std::istream & raw_stream, std::unique_ptr && mutator); + explicit ODBCDriver2ResultReader(const std::string & timezone_, std::istream & raw_stream, std::unique_ptr && mutator); virtual ~ODBCDriver2ResultReader() override = default; virtual bool advanceToNextResultSet() override; diff --git a/driver/format/RowBinaryWithNamesAndTypes.cpp b/driver/format/RowBinaryWithNamesAndTypes.cpp index 8bfed6f8a..f54756320 100644 --- a/driver/format/RowBinaryWithNamesAndTypes.cpp +++ b/driver/format/RowBinaryWithNamesAndTypes.cpp @@ -3,7 +3,7 @@ #include -RowBinaryWithNamesAndTypesResultSet::RowBinaryWithNamesAndTypesResultSet(AmortizedIStreamReader & stream, std::unique_ptr && mutator) +RowBinaryWithNamesAndTypesResultSet::RowBinaryWithNamesAndTypesResultSet(const std::string & timezone, AmortizedIStreamReader & stream, std::unique_ptr && mutator) : ResultSet(stream, std::move(mutator)) { std::uint64_t num_columns = 0; @@ -22,11 +22,10 @@ RowBinaryWithNamesAndTypesResultSet::RowBinaryWithNamesAndTypesResultSet(Amortiz TypeAst ast; if (parser.parse(&ast)) { - columns_info[i].assignTypeInfo(ast); + columns_info[i].assignTypeInfo(ast, timezone); } else { - // Interpret all unparsable types as String. - columns_info[i].type_without_parameters = "String"; + throw std::runtime_error("Unable to read values of an unknown type '" + columns_info[i].type + "'"); } columns_info[i].updateTypeInfo(); @@ -114,14 +113,16 @@ void RowBinaryWithNamesAndTypesResultSet::readValue(Field & dest, ColumnInfo & c constexpr bool convert_on_fetch_conservatively = true; if (convert_on_fetch_conservatively) switch (column_info.type_without_parameters_id) { - case DataSourceTypeId::Date: return readValueAs(dest, column_info); - case DataSourceTypeId::DateTime: return readValueAs(dest, column_info); + case DataSourceTypeId::Date: return readValueUsing( WireTypeDateAsInt (column_info.timezone), dest, column_info); + case DataSourceTypeId::DateTime: return readValueUsing( WireTypeDateTimeAsInt (column_info.timezone), dest, column_info); + case DataSourceTypeId::DateTime64: return readValueUsing( WireTypeDateTime64AsInt (column_info.precision, column_info.timezone), dest, column_info); default: break; // Continue with the next complete switch... } switch (column_info.type_without_parameters_id) { case DataSourceTypeId::Date: return readValueAs>(dest, column_info); case DataSourceTypeId::DateTime: return readValueAs>(dest, column_info); + case DataSourceTypeId::DateTime64: return readValueAs>(dest, column_info); case DataSourceTypeId::Decimal: return readValueAs>(dest, column_info); case DataSourceTypeId::Decimal32: return readValueAs>(dest, column_info); case DataSourceTypeId::Decimal64: return readValueAs>(dest, column_info); @@ -152,14 +153,24 @@ void RowBinaryWithNamesAndTypesResultSet::readValue(WireTypeDateTimeAsInt & dest readPOD(dest.value); } +void RowBinaryWithNamesAndTypesResultSet::readValue(WireTypeDateTime64AsInt & dest, ColumnInfo & column_info) { + readPOD(dest.value); +} + void RowBinaryWithNamesAndTypesResultSet::readValue(DataSourceType & dest, ColumnInfo & column_info) { - WireTypeDateAsInt dest_raw; + WireTypeDateAsInt dest_raw(column_info.timezone); readValue(dest_raw, column_info); value_manip::from_value::template to_value::convert(dest_raw, dest); } void RowBinaryWithNamesAndTypesResultSet::readValue(DataSourceType & dest, ColumnInfo & column_info) { - WireTypeDateTimeAsInt dest_raw; + WireTypeDateTimeAsInt dest_raw(column_info.timezone); + readValue(dest_raw, column_info); + value_manip::from_value::template to_value::convert(dest_raw, dest); +} + +void RowBinaryWithNamesAndTypesResultSet::readValue(DataSourceType & dest, ColumnInfo & column_info) { + WireTypeDateTime64AsInt dest_raw(column_info.precision, column_info.timezone); readValue(dest_raw, column_info); value_manip::from_value::template to_value::convert(dest_raw, dest); } @@ -294,13 +305,13 @@ void RowBinaryWithNamesAndTypesResultSet::readValue(DataSourceType && mutator) - : ResultReader(raw_stream, std::move(mutator)) +RowBinaryWithNamesAndTypesResultReader::RowBinaryWithNamesAndTypesResultReader(const std::string & timezone_, std::istream & raw_stream, std::unique_ptr && mutator) + : ResultReader(timezone_, raw_stream, std::move(mutator)) { if (stream.eof()) return; - result_set = std::make_unique(stream, releaseMutator()); + result_set = std::make_unique(timezone, stream, releaseMutator()); } bool RowBinaryWithNamesAndTypesResultReader::advanceToNextResultSet() { diff --git a/driver/format/RowBinaryWithNamesAndTypes.h b/driver/format/RowBinaryWithNamesAndTypes.h index c0b8e34e3..b74a04d3e 100755 --- a/driver/format/RowBinaryWithNamesAndTypes.h +++ b/driver/format/RowBinaryWithNamesAndTypes.h @@ -8,7 +8,7 @@ class RowBinaryWithNamesAndTypesResultSet : public ResultSet { public: - explicit RowBinaryWithNamesAndTypesResultSet(AmortizedIStreamReader & stream, std::unique_ptr && mutator); + explicit RowBinaryWithNamesAndTypesResultSet(const std::string & timezone, AmortizedIStreamReader & stream, std::unique_ptr && mutator); virtual ~RowBinaryWithNamesAndTypesResultSet() override = default; protected: @@ -29,17 +29,23 @@ class RowBinaryWithNamesAndTypesResultSet void readValue(Field & dest, ColumnInfo & column_info); template - void readValueAs(Field & dest, ColumnInfo & column_info) { - T value; + void readValueUsing(T && value, Field & dest, ColumnInfo & column_info) { readValue(value, column_info); - dest.data = std::move(value); + dest.data = std::forward(value); + } + + template + void readValueAs(Field & dest, ColumnInfo & column_info) { + return readValueUsing(T(), dest, column_info); } void readValue(WireTypeDateAsInt & dest, ColumnInfo & column_info); void readValue(WireTypeDateTimeAsInt & dest, ColumnInfo & column_info); + void readValue(WireTypeDateTime64AsInt & dest, ColumnInfo & column_info); void readValue(DataSourceType< DataSourceTypeId::Date > & dest, ColumnInfo & column_info); void readValue(DataSourceType< DataSourceTypeId::DateTime > & dest, ColumnInfo & column_info); + void readValue(DataSourceType< DataSourceTypeId::DateTime64 > & dest, ColumnInfo & column_info); void readValue(DataSourceType< DataSourceTypeId::Decimal > & dest, ColumnInfo & column_info); void readValue(DataSourceType< DataSourceTypeId::Decimal32 > & dest, ColumnInfo & column_info); void readValue(DataSourceType< DataSourceTypeId::Decimal64 > & dest, ColumnInfo & column_info); @@ -69,7 +75,7 @@ class RowBinaryWithNamesAndTypesResultReader : public ResultReader { public: - explicit RowBinaryWithNamesAndTypesResultReader(std::istream & raw_stream, std::unique_ptr && mutator); + explicit RowBinaryWithNamesAndTypesResultReader(const std::string & timezone, std::istream & raw_stream, std::unique_ptr && mutator); virtual ~RowBinaryWithNamesAndTypesResultReader() override = default; virtual bool advanceToNextResultSet() override; diff --git a/driver/result_set.cpp b/driver/result_set.cpp index 23a7a20d4..78bbe4d68 100644 --- a/driver/result_set.cpp +++ b/driver/result_set.cpp @@ -4,11 +4,34 @@ const std::string::size_type initial_string_capacity_g = std::string{}.capacity(); -void ColumnInfo::assignTypeInfo(const TypeAst & ast) { +void ColumnInfo::assignTypeInfo(const TypeAst & ast, const std::string & default_timezone) { if (ast.meta == TypeAst::Terminal) { type_without_parameters = ast.name; switch (convertUnparametrizedTypeNameToTypeId(type_without_parameters)) { + case DataSourceTypeId::DateTime: { + if (ast.elements.size() != 0 && ast.elements.size() != 1) + throw std::runtime_error("Unexpected DateTime type specification syntax"); + + precision = 0; + timezone = (ast.elements.size() == 1 ? ast.elements.front().name : default_timezone); + + break; + } + + case DataSourceTypeId::DateTime64: { + if (ast.elements.size() != 1 && ast.elements.size() != 2) + throw std::runtime_error("Unexpected DateTime64 type specification syntax"); + + precision = ast.elements.front().size; + timezone = (ast.elements.size() == 2 ? ast.elements.back().name : default_timezone); + + if (precision < 0 || precision > 9) + throw std::runtime_error("Unexpected DateTime64 type specification syntax"); + + break; + } + case DataSourceTypeId::Decimal: { if (ast.elements.size() != 2) throw std::runtime_error("Unexpected Decimal type specification syntax"); @@ -59,7 +82,7 @@ void ColumnInfo::assignTypeInfo(const TypeAst & ast) { } else if (ast.meta == TypeAst::Nullable) { is_nullable = true; - assignTypeInfo(ast.elements.front()); + assignTypeInfo(ast.elements.front(), default_timezone); } else { // Interpret all types with unrecognized ASTs as String. @@ -278,8 +301,9 @@ void ResultSet::retireRow(Row && row) { row_pool.put(std::move(row)); } -ResultReader::ResultReader(std::istream & raw_stream, std::unique_ptr && mutator) - : stream(raw_stream) +ResultReader::ResultReader(const std::string & timezone_, std::istream & raw_stream, std::unique_ptr && mutator) + : timezone(timezone_) + , stream(raw_stream) , result_mutator(std::move(mutator)) { } @@ -299,15 +323,15 @@ std::unique_ptr ResultReader::releaseMutator() { return std::move(result_mutator); } -std::unique_ptr make_result_reader(const std::string & format, std::istream & raw_stream, std::unique_ptr && mutator) { +std::unique_ptr make_result_reader(const std::string & format, const std::string & timezone, std::istream & raw_stream, std::unique_ptr && mutator) { if (format == "ODBCDriver2") { - return std::make_unique(raw_stream, std::move(mutator)); + return std::make_unique(timezone, raw_stream, std::move(mutator)); } else if (format == "RowBinaryWithNamesAndTypes") { if (!isLittleEndian()) throw std::runtime_error("'" + format + "' format is supported only on little-endian platforms"); - return std::make_unique(raw_stream, std::move(mutator)); + return std::make_unique(timezone, raw_stream, std::move(mutator)); } throw std::runtime_error("'" + format + "' format is not supported"); diff --git a/driver/result_set.h b/driver/result_set.h index d886ae6f6..b5119ec5c 100644 --- a/driver/result_set.h +++ b/driver/result_set.h @@ -18,7 +18,7 @@ extern const std::string::size_type initial_string_capacity_g; class ColumnInfo { public: - void assignTypeInfo(const TypeAst & ast); + void assignTypeInfo(const TypeAst & ast, const std::string & default_timezone); void updateTypeInfo(); public: @@ -32,6 +32,7 @@ class ColumnInfo { std::size_t precision = 0; std::size_t scale = 0; bool is_nullable = false; + std::string timezone; }; class Field { @@ -39,6 +40,7 @@ class Field { using DataType = std::variant< DataSourceType< DataSourceTypeId::Date >, DataSourceType< DataSourceTypeId::DateTime >, + DataSourceType< DataSourceTypeId::DateTime64 >, DataSourceType< DataSourceTypeId::Decimal >, DataSourceType< DataSourceTypeId::Decimal32 >, DataSourceType< DataSourceTypeId::Decimal64 >, @@ -61,7 +63,8 @@ class Field { // In case we approach value conversion conservatively... WireTypeAnyAsString, WireTypeDateAsInt, - WireTypeDateTimeAsInt + WireTypeDateTimeAsInt, + WireTypeDateTime64AsInt >; template @@ -131,7 +134,7 @@ class ResultSet { class ResultReader { protected: - explicit ResultReader(std::istream & stream, std::unique_ptr && mutator); + explicit ResultReader(const std::string & timezone_, std::istream & stream, std::unique_ptr && mutator); public: virtual ~ResultReader() = default; @@ -144,12 +147,13 @@ class ResultReader { virtual bool advanceToNextResultSet() = 0; protected: + const std::string timezone; AmortizedIStreamReader stream; std::unique_ptr result_mutator; std::unique_ptr result_set; }; -std::unique_ptr make_result_reader(const std::string & format, std::istream & raw_stream, std::unique_ptr && mutator); +std::unique_ptr make_result_reader(const std::string & format, const std::string & timezone, std::istream & raw_stream, std::unique_ptr && mutator); template SQLRETURN Field::extract(BindingInfo & binding_info, ConversionContext && context) const { diff --git a/driver/statement.cpp b/driver/statement.cpp index 621fd532c..00af70ad1 100755 --- a/driver/statement.cpp +++ b/driver/statement.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -197,7 +198,12 @@ void Statement::requestNextPackOfResultSets(std::unique_ptr && mu throw std::runtime_error(error_message.str()); } - result_reader = make_result_reader(response->get("X-ClickHouse-Format", connection.default_format), *in, std::move(mutator)); + result_reader = make_result_reader( + response->get("X-ClickHouse-Format", connection.default_format), + response->get("X-ClickHouse-Timezone", Poco::Timezone::name()), + *in, std::move(mutator) + ); + ++next_param_set_idx; } diff --git a/driver/utils/type_info.cpp b/driver/utils/type_info.cpp index 1f5545013..66448bd0d 100644 --- a/driver/utils/type_info.cpp +++ b/driver/utils/type_info.cpp @@ -7,12 +7,7 @@ const std::map types_g = { {"UInt8", TypeInfo {"TINYINT", true, SQL_TINYINT, 3, 1}}, {"UInt16", TypeInfo {"SMALLINT", true, SQL_SMALLINT, 5, 2}}, - {"UInt32", - TypeInfo {"INT", - true, - SQL_BIGINT /* was SQL_INTEGER */, - 10, - 4}}, // With perl, python ODBC drivers INT is uint32 and it cant store values bigger than 2147483647: 2147483648 -> -2147483648 4294967295 -> -1 + {"UInt32", TypeInfo {"INT", true, SQL_BIGINT /* was SQL_INTEGER */, 10, 4}}, // With perl, python ODBC drivers INT is uint32 and it cant store values bigger than 2147483647: 2147483648 -> -2147483648 4294967295 -> -1 {"UInt32", TypeInfo {"INT", true, SQL_INTEGER, 10, 4}}, {"UInt64", TypeInfo {"BIGINT", true, SQL_BIGINT, 20, 8}}, {"Int8", TypeInfo {"TINYINT", false, SQL_TINYINT, 1 + 3, 1}}, // one char for sign @@ -27,19 +22,19 @@ const std::map types_g = { {"FixedString", TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, {"Date", TypeInfo {"DATE", true, SQL_TYPE_DATE, 10, 6}}, {"DateTime", TypeInfo {"TIMESTAMP", true, SQL_TYPE_TIMESTAMP, 19, 16}}, + {"DateTime64", TypeInfo {"TIMESTAMP", true, SQL_TYPE_TIMESTAMP, 29, 16}}, {"Array", TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, - - {"LowCardinality(String)", - TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, // todo: remove - {"LowCardinality(FixedString)", - TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, // todo: remove - {"Nothing", TypeInfo {"NULL", true, SQL_TYPE_NULL, 1, 1}}, + + // TODO: remove these. + {"LowCardinality(String)", TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}}, + {"LowCardinality(FixedString)", TypeInfo {"TEXT", true, SQL_VARCHAR, TypeInfo::string_max_size, TypeInfo::string_max_size}} }; DataSourceTypeId convertUnparametrizedTypeNameToTypeId(const std::string & type_name) { if (Poco::icompare(type_name, "Date") == 0) return DataSourceTypeId::Date; else if (Poco::icompare(type_name, "DateTime") == 0) return DataSourceTypeId::DateTime; + else if (Poco::icompare(type_name, "DateTime64") == 0) return DataSourceTypeId::DateTime64; else if (Poco::icompare(type_name, "Decimal") == 0) return DataSourceTypeId::Decimal; else if (Poco::icompare(type_name, "Decimal32") == 0) return DataSourceTypeId::Decimal32; else if (Poco::icompare(type_name, "Decimal64") == 0) return DataSourceTypeId::Decimal64; @@ -77,6 +72,7 @@ std::string convertTypeIdToUnparametrizedCanonicalTypeName(DataSourceTypeId type switch (type_id) { case DataSourceTypeId::Date: return "Date"; case DataSourceTypeId::DateTime: return "DateTime"; + case DataSourceTypeId::DateTime64: return "DateTime64"; case DataSourceTypeId::Decimal: return "Decimal"; case DataSourceTypeId::Decimal32: return "Decimal32"; case DataSourceTypeId::Decimal64: return "Decimal64"; diff --git a/driver/utils/type_info.h b/driver/utils/type_info.h index d895bd4fc..9e77c7a75 100755 --- a/driver/utils/type_info.h +++ b/driver/utils/type_info.h @@ -56,6 +56,7 @@ enum class DataSourceTypeId { Unknown, Date, DateTime, + DateTime64, Decimal, Decimal32, Decimal64, @@ -404,18 +405,42 @@ struct WireTypeAnyAsString using SimpleTypeWrapper::SimpleTypeWrapper; }; -// Date stored exactly as it is represented on wire in RowBinaryWithNamesAndTypes format. -struct WireTypeDateAsInt - : public SimpleTypeWrapper -{ - using SimpleTypeWrapper::SimpleTypeWrapper; +struct WireTypeDateAsInt { + explicit WireTypeDateAsInt(const std::string & timezone_) + : timezone(&timezone_) + { + } + + using ContainerIntType = std::uint16_t; + + ContainerIntType value = 0; + const std::string * timezone; }; -// DateTime stored exactly as it is represented on wire in RowBinaryWithNamesAndTypes format. -struct WireTypeDateTimeAsInt - : public SimpleTypeWrapper -{ - using SimpleTypeWrapper::SimpleTypeWrapper; +struct WireTypeDateTimeAsInt { + explicit WireTypeDateTimeAsInt(const std::string & timezone_) + : timezone(&timezone_) + { + } + + using ContainerIntType = std::uint32_t; + + ContainerIntType value = 0; + const std::string * timezone; +}; + +struct WireTypeDateTime64AsInt { + explicit WireTypeDateTime64AsInt(std::int16_t precision_, const std::string & timezone_) + : precision(precision_) + , timezone(&timezone_) + { + } + + using ContainerIntType = std::uint64_t; + + ContainerIntType value = 0; + std::int16_t precision; + const std::string * timezone; }; template struct DataSourceType; // Leave unimplemented for general case. @@ -434,6 +459,13 @@ struct DataSourceType using SimpleTypeWrapper::SimpleTypeWrapper; }; +template <> +struct DataSourceType + : public SimpleTypeWrapper +{ + using SimpleTypeWrapper::SimpleTypeWrapper; +}; + template <> struct DataSourceType { // An integer type big enough to hold the integer value that is built from all @@ -948,7 +980,7 @@ namespace value_manip { dest.second = 0; dest.fraction = 0; } - else if (src.size() == 19) { + else if (src.size() >= 19 && src.size() <= 29) { dest.year = (src[0] - '0') * 1000 + (src[1] - '0') * 100 + (src[2] - '0') * 10 + (src[3] - '0'); dest.month = (src[5] - '0') * 10 + (src[6] - '0'); dest.day = (src[8] - '0') * 10 + (src[9] - '0'); @@ -956,6 +988,14 @@ namespace value_manip { dest.minute = (src[14] - '0') * 10 + (src[15] - '0'); dest.second = (src[17] - '0') * 10 + (src[18] - '0'); dest.fraction = 0; + + for (std::size_t i = 20; i < 29; ++i) { + dest.fraction *= 10; + + if (i < src.size()) { + dest.fraction += (src[i] - '0'); + } + } } else throw std::runtime_error("Cannot interpret '" + src + "' as DateTime"); @@ -1766,6 +1806,9 @@ namespace value_manip { using DestinationType = DataSourceType; static inline void convert(const SourceType & src, DestinationType & dest) { + + // TODO: convert time according to src.timezone + std::time_t time = src.value; time = time * 24 * 60 * 60; // Now it's seconds since epoch. const auto & tm = *std::localtime(&time); @@ -1793,6 +1836,9 @@ namespace value_manip { using DestinationType = DataSourceType; static inline void convert(const SourceType & src, DestinationType & dest) { + + // TODO: convert time according to src.timezone + std::time_t time = src.value; const auto & tm = *std::localtime(&time); @@ -1806,6 +1852,40 @@ namespace value_manip { } }; + template <> + struct from_value { + using SourceType = WireTypeDateTime64AsInt; + + template + struct to_value { + static inline void convert(const SourceType & src, DestinationType & dest) { + convert_via_proxy>(src, dest); + } + }; + }; + + template <> + struct from_value::to_value> { + using DestinationType = DataSourceType; + + static inline void convert(const SourceType & src, DestinationType & dest) { + static constexpr SQLUINTEGER pow10[] = {1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000}; + + // TODO: convert time according to src.timezone + + std::time_t time = src.value / pow10[src.precision]; + const auto & tm = *std::localtime(&time); + + dest.value.year = 1900 + tm.tm_year; + dest.value.month = 1 + tm.tm_mon; + dest.value.day = tm.tm_mday; + dest.value.hour = tm.tm_hour; + dest.value.minute = tm.tm_min; + dest.value.second = tm.tm_sec; + dest.value.fraction = src.value % pow10[src.precision]; + } + }; + template struct to_buffer { template diff --git a/driver/utils/type_parser.cpp b/driver/utils/type_parser.cpp index f8942db6b..08bf5b468 100644 --- a/driver/utils/type_parser.cpp +++ b/driver/utils/type_parser.cpp @@ -91,6 +91,19 @@ TypeParser::Token TypeParser::nextToken() { default: { const char * st = cur_; + if (*cur_ == '"' || *cur_ == '\'') { + for (++cur_; cur_ < end_; ++cur_) { + if (*cur_ == *st) { + break; + } + } + + if (cur_ == end_) + return Token {Token::Invalid, std::string()}; + + return Token {Token::Name, std::string(st + 1, cur_++)}; + } + if (isalpha(*cur_)) { for (; cur_ < end_; ++cur_) { if (!isalpha(*cur_) && !isdigit(*cur_)) { From 8b983abb0b92ad12bb5365c57b34bdbd5bd59e7b Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Sat, 24 Jul 2021 17:46:18 +0400 Subject: [PATCH 14/20] Fix/enrich/implement Date/DateTime/DateTime64 handling Add Date/DateTime/DateTime64 testing --- driver/test/CMakeLists.txt | 1 + driver/test/common_utils.h | 34 ++++++ driver/test/datetime_it.cpp | 227 ++++++++++++++++++++++++++++++++++++ driver/test/misc_it.cpp | 34 +++--- driver/utils/type_info.h | 54 +++++---- 5 files changed, 309 insertions(+), 41 deletions(-) create mode 100755 driver/test/datetime_it.cpp diff --git a/driver/test/CMakeLists.txt b/driver/test/CMakeLists.txt index da332c8e9..f6940c704 100644 --- a/driver/test/CMakeLists.txt +++ b/driver/test/CMakeLists.txt @@ -67,6 +67,7 @@ function (declare_odbc_test_targets libname UNICODE) ${PROJECT_SOURCE_DIR}/driver/utils/conversion_std.h ${PROJECT_SOURCE_DIR}/driver/utils/conversion_icu.h misc_it.cpp + datetime_it.cpp column_bindings_it.cpp statement_parameter_bindings_it.cpp statement_parameters_it.cpp diff --git a/driver/test/common_utils.h b/driver/test/common_utils.h index e95864e70..69c07e7c9 100644 --- a/driver/test/common_utils.h +++ b/driver/test/common_utils.h @@ -2,11 +2,18 @@ #include "driver/platform/platform.h" +#include + #include #include +#include +#include #include #include +#include +#include + #if defined(BUILD_TYPE_RELEASE) # define ENABLE_FOR_OPTIMIZED_BUILDS_ONLY(test) test #else @@ -39,3 +46,30 @@ std::cout << "\tLatency: " << str.str() << " milliseconds per iteration" << std::endl; \ } \ } + +inline std::optional get_env_var(const std::string & name) { + if (Poco::Environment::has(name)) { + return Poco::Environment::get("TZ"); + } + + return {}; +} + +inline void unset_env_var(const std::string & name) { +#ifdef _win_ + Poco::Environment::set(name, ""); +#else + if (unsetenv(name.c_str()) != 0) { + throw std::runtime_error("Failed to unset environment variable " + name + ": " + std::strerror(errno)); + } +#endif +} + +inline void set_env_var(const std::string & name, const std::optional & value) { + if (value.has_value()) { + Poco::Environment::set(name, value.value()); + } + else { + unset_env_var(name); + } +} diff --git a/driver/test/datetime_it.cpp b/driver/test/datetime_it.cpp new file mode 100755 index 000000000..1ff6b0792 --- /dev/null +++ b/driver/test/datetime_it.cpp @@ -0,0 +1,227 @@ +#include "driver/platform/platform.h" +#include "driver/test/client_utils.h" +#include "driver/test/client_test_base.h" + +#include +#include + +#include + +class DateTime + : public ClientTestWithParamBase< + std::tuple< + std::string, // parameter set name + + // TODO: remove this once the formats behave identically. + std::string, // format to use + std::string, // local timezone to use + + std::string, // expression for SELECT + SQLSMALLINT, // expected reported column type + std::string, // value, when retrieved as string + SQL_TIMESTAMP_STRUCT // value, when retrieved as SQL_TIMESTAMP_STRUCT + > + > +{ +}; + +bool operator== (const SQL_DATE_STRUCT & lhs, const SQL_DATE_STRUCT & rhs) { + return ( + lhs.year == rhs.year && + lhs.month == rhs.month && + lhs.day == rhs.day + ); +} + +bool operator== (const SQL_TIME_STRUCT & lhs, const SQL_TIME_STRUCT & rhs) { + return ( + lhs.hour == rhs.hour && + lhs.minute == rhs.minute && + lhs.second == rhs.second + ); +} + +bool operator== (const SQL_TIMESTAMP_STRUCT & lhs, const SQL_TIMESTAMP_STRUCT & rhs) { + return ( + lhs.year == rhs.year && + lhs.month == rhs.month && + lhs.day == rhs.day && + lhs.hour == rhs.hour && + lhs.minute == rhs.minute && + lhs.second == rhs.second && + lhs.fraction == rhs.fraction + ); +} + +TEST_P(DateTime, GetData) { + const auto & [ + name, // unused here + format, + local_tz, + expr, + expected_sql_type, + expected_str_val, + expected_timestamp_val + ] = GetParam(); + + const SQL_DATE_STRUCT expected_date_val = { + expected_timestamp_val.year, + expected_timestamp_val.month, + expected_timestamp_val.day + }; + + const SQL_TIME_STRUCT expected_time_val = { + expected_timestamp_val.hour, + expected_timestamp_val.minute, + expected_timestamp_val.second + }; + + const auto orig_local_tz = get_env_var("TZ"); + set_env_var("TZ", local_tz); + try { + + const std::string query_orig = "SELECT " + expr + " AS col FORMAT " + format; + + const auto query = fromUTF8(query_orig); + auto * query_wptr = const_cast(query.c_str()); + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecDirect(hstmt, query_wptr, SQL_NTS)); + ODBC_CALL_ON_STMT_THROW(hstmt, SQLFetch(hstmt)); + + { + SQLLEN sql_type = SQL_TYPE_NULL; + ODBC_CALL_ON_STMT_THROW(hstmt, SQLColAttribute(hstmt, 1, SQL_DESC_TYPE, NULL, 0, NULL, &sql_type)); + EXPECT_EQ(sql_type, expected_sql_type); + } + + { + SQLTCHAR col[64] = {}; + SQLLEN col_ind = 0; + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLGetData( + hstmt, + 1, + getCTypeFor(), + &col, + sizeof(col), + &col_ind + )); + + EXPECT_EQ(toUTF8(col), expected_str_val); + } + + if (format != "RowBinaryWithNamesAndTypes" || expected_sql_type == SQL_TYPE_DATE) { + SQL_DATE_STRUCT col = {}; + SQLLEN col_ind = 0; + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLGetData( + hstmt, + 1, + getCTypeFor(), + &col, + sizeof(col), + &col_ind + )); + + EXPECT_EQ(col, expected_date_val); + } + + if (format != "RowBinaryWithNamesAndTypes") { + SQL_TIME_STRUCT col = {}; + SQLLEN col_ind = 0; + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLGetData( + hstmt, + 1, + getCTypeFor(), + &col, + sizeof(col), + &col_ind + )); + + EXPECT_EQ(col, expected_time_val); + } + + if (format != "RowBinaryWithNamesAndTypes" || expected_sql_type != SQL_TYPE_DATE) { + SQL_TIMESTAMP_STRUCT col = {}; + SQLLEN col_ind = 0; + + ODBC_CALL_ON_STMT_THROW(hstmt, SQLGetData( + hstmt, + 1, + getCTypeFor(), + &col, + sizeof(col), + &col_ind + )); + + EXPECT_EQ(col, expected_timestamp_val); + } + + } + catch (...) { + try { + set_env_var("TZ", orig_local_tz); + } + catch (const std::exception & ex) { + std::cerr << ex.what() << std::endl; + } + + throw; + } + + set_env_var("TZ", orig_local_tz); +} + +INSTANTIATE_TEST_SUITE_P( + MiscellaneousTest, + DateTime, + ::testing::Values( + std::make_tuple("Date", "ODBCDriver2", "Europe/Moscow", + "toDate('2020-03-25')", SQL_TYPE_DATE, + "2020-03-25", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 0, 0, 0, 0} + ), + std::make_tuple("DateTime", "ODBCDriver2", "Europe/Moscow", + "toDateTime('2020-03-25 12:11:22')", SQL_TYPE_TIMESTAMP, + "2020-03-25 12:11:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 0} + ), + std::make_tuple("DateTime_TZ", "ODBCDriver2", "Europe/Moscow", + "toDateTime('2020-03-25 12:11:22', 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, + "2020-03-25 12:11:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 0} + ), + std::make_tuple("DateTime64_0", "ODBCDriver2", "Europe/Moscow", + "toDateTime64('2020-03-25 12:11:22.123456789', 0)", SQL_TYPE_TIMESTAMP, + "2020-03-25 12:11:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 0} + ), + std::make_tuple("DateTime64_4", "ODBCDriver2", "Europe/Moscow", + "toDateTime64('2020-03-25 12:11:22.123456789', 4)", SQL_TYPE_TIMESTAMP, + "2020-03-25 12:11:22.1234", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 123400000} + ), + std::make_tuple("DateTime64_9", "ODBCDriver2", "Europe/Moscow", + "toDateTime64('2020-03-25 12:11:22.123456789', 9)", SQL_TYPE_TIMESTAMP, + "2020-03-25 12:11:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 123456789} + ), + std::make_tuple("DateTime64_9_TZ", "ODBCDriver2", "Europe/Moscow", + "toDateTime64('2020-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, + "2020-03-25 12:11:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 123456789} + ), + + // TODO: remove this once the formats behave identically. + + std::make_tuple("Date", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + "toDate('2020-03-25')", SQL_TYPE_DATE, + "2020-03-25", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 0, 0, 0, 0} + ), + std::make_tuple("DateTime_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + "toDateTime('2020-03-25 12:11:22', 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, + "2020-03-25 09:26:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 9, 26, 22, 0} + ), + std::make_tuple("DateTime64_9_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + "toDateTime64('2020-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, + "2020-03-25 09:26:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 9, 26, 22, 123456789} + ) + ), + [] (const auto & param_info) { + return std::get<0>(param_info.param) + "_over_" + std::get<1>(param_info.param); + } +); diff --git a/driver/test/misc_it.cpp b/driver/test/misc_it.cpp index 5d2d54ba9..1979a188c 100755 --- a/driver/test/misc_it.cpp +++ b/driver/test/misc_it.cpp @@ -102,7 +102,7 @@ TEST_F(MiscellaneousTest, SQLGetData_ZeroOutputBufferSize) { } TEST_F(MiscellaneousTest, NullableNothing) { - const std::string query_orig = "SELECT NULL as col"; + const std::string query_orig = "SELECT NULL AS col"; const auto query = fromUTF8(query_orig); auto * query_wptr = const_cast(query.c_str()); @@ -124,11 +124,14 @@ enum class FailOn { Never }; -// First parameter - parameter set name. -// Second parameter - extra name=value semicolon-separated string to append to the connection string. -// Third parameter - when to expect the failure, if any. class ConnectionFailureReporing - : public ClientTestWithParamBase> + : public ClientTestWithParamBase< + std::tuple< + std::string, // parameter set name + std::string, // extra name=value semicolon-separated string to append to the connection string + FailOn // when to expect the failure, if any + > + > { private: using Base = ClientTestWithParamBase>; @@ -231,13 +234,17 @@ INSTANTIATE_TEST_SUITE_P( } ); -// First parameter - original "huge" integer type. -// Second parameter - a tuple of: -// First parameter - parameter set name. -// Second parameter - extra name=value semicolon-separated string to append to the connection string. -// Third parameter - expected reported column type. class HugeIntTypeReporting - : public ClientTestWithParamBase>> + : public ClientTestWithParamBase< + std::tuple< + std::string, // original "huge" integer type + std::tuple< + std::string, // parameter set name + std::string, // extra name=value semicolon-separated string to append to the connection string + SQLSMALLINT // expected reported column type + > + > + > { private: using Base = ClientTestWithParamBase>>; @@ -275,9 +282,7 @@ TEST_P(HugeIntTypeReporting, Check) { ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecDirect(hstmt, query_wptr, SQL_NTS)); SQLLEN sql_type = SQL_TYPE_NULL; - ODBC_CALL_ON_STMT_THROW(hstmt, SQLColAttribute(hstmt, 1, SQL_DESC_TYPE, NULL, 0, NULL, &sql_type)); - ASSERT_EQ(sql_type, expected_sql_type); } @@ -286,7 +291,10 @@ INSTANTIATE_TEST_SUITE_P( HugeIntTypeReporting, ::testing::Combine( ::testing::Values( + + // TODO: uncomment each type once its support is implemented. "UInt64"/*, "Int128", "UInt128", "Int256", "UInt256"*/ + ), ::testing::Values( std::make_tuple("HugeIntAsString_Default", "", SQL_BIGINT), diff --git a/driver/utils/type_info.h b/driver/utils/type_info.h index 9e77c7a75..56ca14253 100755 --- a/driver/utils/type_info.h +++ b/driver/utils/type_info.h @@ -938,8 +938,8 @@ namespace value_manip { using DestinationType = SQL_DATE_STRUCT; static inline void convert(const SourceType & src, DestinationType & dest) { - if (src.size() != 10) - throw std::runtime_error("Cannot interpret '" + src + "' as Date"); + if (src.size() != 10 && (src.size() < 19 || src.size() > 29)) + throw std::runtime_error("Cannot interpret '" + src + "' as DATE"); dest.year = (src[0] - '0') * 1000 + (src[1] - '0') * 100 + (src[2] - '0') * 10 + (src[3] - '0'); dest.month = (src[5] - '0') * 10 + (src[6] - '0'); @@ -954,14 +954,18 @@ namespace value_manip { using DestinationType = SQL_TIME_STRUCT; static inline void convert(const SourceType & src, DestinationType & dest) { - if constexpr (std::is_same_v) { - std::memcpy(&dest, &src, sizeof(dest)); + if (src.size() != 10 && (src.size() < 19 || src.size() > 29)) + throw std::runtime_error("Cannot interpret '" + src + "' as TIME"); + + if (src.size() > 10) { + dest.hour = (src[11] - '0') * 10 + (src[12] - '0'); + dest.minute = (src[14] - '0') * 10 + (src[15] - '0'); + dest.second = (src[17] - '0') * 10 + (src[18] - '0'); } else { - throw std::runtime_error("conversion not supported"); - - // TODO: implement? - + dest.hour = 0; + dest.minute = 0; + dest.second = 0; } } }; @@ -971,34 +975,28 @@ namespace value_manip { using DestinationType = SQL_TIMESTAMP_STRUCT; static inline void convert(const SourceType & src, DestinationType & dest) { - if (src.size() == 10) { - dest.year = (src[0] - '0') * 1000 + (src[1] - '0') * 100 + (src[2] - '0') * 10 + (src[3] - '0'); - dest.month = (src[5] - '0') * 10 + (src[6] - '0'); - dest.day = (src[8] - '0') * 10 + (src[9] - '0'); - dest.hour = 0; - dest.minute = 0; - dest.second = 0; - dest.fraction = 0; - } - else if (src.size() >= 19 && src.size() <= 29) { - dest.year = (src[0] - '0') * 1000 + (src[1] - '0') * 100 + (src[2] - '0') * 10 + (src[3] - '0'); - dest.month = (src[5] - '0') * 10 + (src[6] - '0'); - dest.day = (src[8] - '0') * 10 + (src[9] - '0'); + if (src.size() != 10 && (src.size() < 19 || src.size() > 29)) + throw std::runtime_error("Cannot interpret '" + src + "' as TIMESTAMP"); + + dest.year = (src[0] - '0') * 1000 + (src[1] - '0') * 100 + (src[2] - '0') * 10 + (src[3] - '0'); + dest.month = (src[5] - '0') * 10 + (src[6] - '0'); + dest.day = (src[8] - '0') * 10 + (src[9] - '0'); + + if (src.size() >= 19) { dest.hour = (src[11] - '0') * 10 + (src[12] - '0'); dest.minute = (src[14] - '0') * 10 + (src[15] - '0'); dest.second = (src[17] - '0') * 10 + (src[18] - '0'); dest.fraction = 0; - for (std::size_t i = 20; i < 29; ++i) { - dest.fraction *= 10; - - if (i < src.size()) { - dest.fraction += (src[i] - '0'); + if (src.size() > 20) { + for (std::size_t i = 20; i < 29; ++i) { + dest.fraction *= 10; + if (i < src.size()) { + dest.fraction += (src[i] - '0'); + } } } } - else - throw std::runtime_error("Cannot interpret '" + src + "' as DateTime"); normalize_date(dest); } From 376c471f08ea35eedce45e4832d79e2e5c0c5797 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Mon, 26 Jul 2021 13:50:21 +0400 Subject: [PATCH 15/20] Change DateTime64 deserialization in RowBinaryWithNamesAndTypes to std::int64_t --- driver/test/datetime_it.cpp | 8 ++++++++ driver/utils/type_info.h | 12 +++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/driver/test/datetime_it.cpp b/driver/test/datetime_it.cpp index 1ff6b0792..5d0ae4136 100755 --- a/driver/test/datetime_it.cpp +++ b/driver/test/datetime_it.cpp @@ -219,7 +219,15 @@ INSTANTIATE_TEST_SUITE_P( std::make_tuple("DateTime64_9_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", "toDateTime64('2020-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, "2020-03-25 09:26:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 9, 26, 22, 123456789} + )/*, + + // TODO: uncomment once the target ClickHouse server is 21.4+ + + std::make_tuple("DateTime64_9_TZ_pre_epoch", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + "toDateTime64('1955-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, + "1955-03-25 09:26:22.123456789", SQL_TIMESTAMP_STRUCT{1955, 3, 25, 9, 26, 22, 123456789} ) + */ ), [] (const auto & param_info) { return std::get<0>(param_info.param) + "_over_" + std::get<1>(param_info.param); diff --git a/driver/utils/type_info.h b/driver/utils/type_info.h index 56ca14253..5e65004d1 100755 --- a/driver/utils/type_info.h +++ b/driver/utils/type_info.h @@ -436,7 +436,7 @@ struct WireTypeDateTime64AsInt { { } - using ContainerIntType = std::uint64_t; + using ContainerIntType = std::int64_t; ContainerIntType value = 0; std::int16_t precision; @@ -1871,7 +1871,13 @@ namespace value_manip { // TODO: convert time according to src.timezone - std::time_t time = src.value / pow10[src.precision]; + const auto secs = src.value / pow10[src.precision]; + const auto fraction = std::abs(src.value % pow10[src.precision]); + + if (secs < 0 || secs > std::numeric_limits::max()) + throw std::runtime_error("Cannot represent " + std::to_string(secs) + " seconds since the Unix epoch as SQL_TIMESTAMP_STRUCT"); + + std::time_t time = secs; const auto & tm = *std::localtime(&time); dest.value.year = 1900 + tm.tm_year; @@ -1880,7 +1886,7 @@ namespace value_manip { dest.value.hour = tm.tm_hour; dest.value.minute = tm.tm_min; dest.value.second = tm.tm_sec; - dest.value.fraction = src.value % pow10[src.precision]; + dest.value.fraction = fraction; } }; From 67b36ef86ebea0fd5372b219f5c84c9771b1eacc Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Mon, 26 Jul 2021 17:51:42 +0400 Subject: [PATCH 16/20] Use safe localtime --- driver/driver.cpp | 14 ++++++++++---- driver/utils/type_info.h | 32 +++++++++++++++++++++----------- driver/utils/utils.h | 18 ++++++++++++++++++ 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/driver/driver.cpp b/driver/driver.cpp index 426cde725..9ab61aacd 100755 --- a/driver/driver.cpp +++ b/driver/driver.cpp @@ -97,9 +97,12 @@ void Driver::writeLogMessagePrefix(std::ostream & stream) { void Driver::writeLogSessionStart(std::ostream & stream) { stream << "==================== ODBC Driver logging session started"; { - auto t = std::time(nullptr); + std::tm tm = {}; + const auto time = std::time(nullptr); + toLocalTime(time, tm); + char mbstr[100] = {}; - if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", std::localtime(&t))) + if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", &tm)) stream << " (" << mbstr << ")"; } stream << " ====================" << std::endl; @@ -127,9 +130,12 @@ void Driver::writeLogSessionStart(std::ostream & stream) { void Driver::writeLogSessionEnd(std::ostream & stream) { stream << "==================== ODBC Driver logging session ended"; { - auto t = std::time(nullptr); + std::tm tm = {}; + const auto time = std::time(nullptr); + toLocalTime(time, tm); + char mbstr[100] = {}; - if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", std::localtime(&t))) + if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", &tm)) stream << " (" << mbstr << ")"; } stream << " ====================" << std::endl; diff --git a/driver/utils/type_info.h b/driver/utils/type_info.h index 5e65004d1..9d777abb7 100755 --- a/driver/utils/type_info.h +++ b/driver/utils/type_info.h @@ -1804,12 +1804,15 @@ namespace value_manip { using DestinationType = DataSourceType; static inline void convert(const SourceType & src, DestinationType & dest) { + std::tm tm = {}; - // TODO: convert time according to src.timezone + { + // TODO: convert time according to src.timezone - std::time_t time = src.value; - time = time * 24 * 60 * 60; // Now it's seconds since epoch. - const auto & tm = *std::localtime(&time); + std::time_t time = src.value; + time = time * 24 * 60 * 60; // Now it's seconds since epoch. + toLocalTime(time, tm); + } dest.value.year = 1900 + tm.tm_year; dest.value.month = 1 + tm.tm_mon; @@ -1834,11 +1837,14 @@ namespace value_manip { using DestinationType = DataSourceType; static inline void convert(const SourceType & src, DestinationType & dest) { + std::tm tm = {}; - // TODO: convert time according to src.timezone + { + // TODO: convert time according to src.timezone - std::time_t time = src.value; - const auto & tm = *std::localtime(&time); + const std::time_t time = src.value; + toLocalTime(time, tm); + } dest.value.year = 1900 + tm.tm_year; dest.value.month = 1 + tm.tm_mon; @@ -1869,16 +1875,20 @@ namespace value_manip { static inline void convert(const SourceType & src, DestinationType & dest) { static constexpr SQLUINTEGER pow10[] = {1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000}; - // TODO: convert time according to src.timezone - const auto secs = src.value / pow10[src.precision]; const auto fraction = std::abs(src.value % pow10[src.precision]); if (secs < 0 || secs > std::numeric_limits::max()) throw std::runtime_error("Cannot represent " + std::to_string(secs) + " seconds since the Unix epoch as SQL_TIMESTAMP_STRUCT"); - std::time_t time = secs; - const auto & tm = *std::localtime(&time); + std::tm tm = {}; + + { + // TODO: convert time according to src.timezone + + const std::time_t time = secs; + toLocalTime(time, tm); + } dest.value.year = 1900 + tm.tm_year; dest.value.month = 1 + tm.tm_mon; diff --git a/driver/utils/utils.h b/driver/utils/utils.h index c08d90708..69bc697a3 100755 --- a/driver/utils/utils.h +++ b/driver/utils/utils.h @@ -26,13 +26,17 @@ #include #include #include +#include #include #include #include #include #include +#define __STDC_WANT_LIB_EXT1__ 1 + #include +#include #ifdef _win_ # define stack_alloc _alloca @@ -118,6 +122,20 @@ inline bool isLittleEndian() noexcept { return (i.i8[0] == 0xEF); } +inline void toLocalTime(const std::time_t & src, std::tm & dest) { +#ifdef _win_ + const auto err = localtime_s(&dest, &src); +#elif defined(__STDC_LIB_EXT1__) + const auto err = localtime_s(&src, &dest); +#else + auto * res = localtime_r(&src, &dest); + const auto err = (res == &dest ? 0 : errno); +#endif + + if (err) + throw std::runtime_error("Failed to convert time: " + std::string(std::strerror(err))); +} + inline bool isYes(std::string str) { Poco::trimInPlace(str); Poco::UTF8::toLowerInPlace(str); From 3a24d04f8fa3b7034627a70ab286046887f63d3c Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Mon, 2 Aug 2021 22:25:50 +0400 Subject: [PATCH 17/20] Fix exception handling vs temporary statement cleanup --- driver/connection.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/driver/connection.cpp b/driver/connection.cpp index bb3ae887f..5577e5b98 100644 --- a/driver/connection.cpp +++ b/driver/connection.cpp @@ -480,7 +480,15 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v void Connection::verifyConnection() { LOG("Verifying connection and credentials..."); auto & statement = allocateChild(); - statement.executeQuery("SELECT 1"); + + try { + statement.executeQuery("SELECT 1"); + } + catch (...) { + statement.deallocateSelf(); + throw; + } + statement.deallocateSelf(); } From be6f4dfa15e345f6a55b60541233dccd5c1f7dd5 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Mon, 2 Aug 2021 22:26:16 +0400 Subject: [PATCH 18/20] Change test parameters from a tuple to struct --- driver/test/datetime_it.cpp | 113 +++++++++++++++++------------------- 1 file changed, 53 insertions(+), 60 deletions(-) diff --git a/driver/test/datetime_it.cpp b/driver/test/datetime_it.cpp index 5d0ae4136..0f4059c4a 100755 --- a/driver/test/datetime_it.cpp +++ b/driver/test/datetime_it.cpp @@ -7,21 +7,22 @@ #include +struct DateTimeParams { + std::string name; // parameter set name + + // TODO: remove this once the formats behave identically. + std::string format; // format to use + std::string local_tz; // local timezone to use + + std::string expr; // expression for SELECT + SQLSMALLINT expected_sql_type; // expected reported column type + std::string expected_str_val; // value, when retrieved as string + SQL_TIMESTAMP_STRUCT expected_timestamp_val; // value, when retrieved as SQL_TIMESTAMP_STRUCT + +}; + class DateTime - : public ClientTestWithParamBase< - std::tuple< - std::string, // parameter set name - - // TODO: remove this once the formats behave identically. - std::string, // format to use - std::string, // local timezone to use - - std::string, // expression for SELECT - SQLSMALLINT, // expected reported column type - std::string, // value, when retrieved as string - SQL_TIMESTAMP_STRUCT // value, when retrieved as SQL_TIMESTAMP_STRUCT - > - > + : public ClientTestWithParamBase { }; @@ -54,33 +55,25 @@ bool operator== (const SQL_TIMESTAMP_STRUCT & lhs, const SQL_TIMESTAMP_STRUCT & } TEST_P(DateTime, GetData) { - const auto & [ - name, // unused here - format, - local_tz, - expr, - expected_sql_type, - expected_str_val, - expected_timestamp_val - ] = GetParam(); + const auto & params = GetParam(); const SQL_DATE_STRUCT expected_date_val = { - expected_timestamp_val.year, - expected_timestamp_val.month, - expected_timestamp_val.day + params.expected_timestamp_val.year, + params.expected_timestamp_val.month, + params.expected_timestamp_val.day }; const SQL_TIME_STRUCT expected_time_val = { - expected_timestamp_val.hour, - expected_timestamp_val.minute, - expected_timestamp_val.second + params.expected_timestamp_val.hour, + params.expected_timestamp_val.minute, + params.expected_timestamp_val.second }; const auto orig_local_tz = get_env_var("TZ"); - set_env_var("TZ", local_tz); + set_env_var("TZ", params.local_tz); try { - const std::string query_orig = "SELECT " + expr + " AS col FORMAT " + format; + const std::string query_orig = "SELECT " + params.expr + " AS col FORMAT " + params.format; const auto query = fromUTF8(query_orig); auto * query_wptr = const_cast(query.c_str()); @@ -91,7 +84,7 @@ TEST_P(DateTime, GetData) { { SQLLEN sql_type = SQL_TYPE_NULL; ODBC_CALL_ON_STMT_THROW(hstmt, SQLColAttribute(hstmt, 1, SQL_DESC_TYPE, NULL, 0, NULL, &sql_type)); - EXPECT_EQ(sql_type, expected_sql_type); + EXPECT_EQ(sql_type, params.expected_sql_type); } { @@ -107,10 +100,10 @@ TEST_P(DateTime, GetData) { &col_ind )); - EXPECT_EQ(toUTF8(col), expected_str_val); + EXPECT_EQ(toUTF8(col), params.expected_str_val); } - if (format != "RowBinaryWithNamesAndTypes" || expected_sql_type == SQL_TYPE_DATE) { + if (params.format != "RowBinaryWithNamesAndTypes" || params.expected_sql_type == SQL_TYPE_DATE) { SQL_DATE_STRUCT col = {}; SQLLEN col_ind = 0; @@ -126,7 +119,7 @@ TEST_P(DateTime, GetData) { EXPECT_EQ(col, expected_date_val); } - if (format != "RowBinaryWithNamesAndTypes") { + if (params.format != "RowBinaryWithNamesAndTypes") { SQL_TIME_STRUCT col = {}; SQLLEN col_ind = 0; @@ -142,7 +135,7 @@ TEST_P(DateTime, GetData) { EXPECT_EQ(col, expected_time_val); } - if (format != "RowBinaryWithNamesAndTypes" || expected_sql_type != SQL_TYPE_DATE) { + if (params.format != "RowBinaryWithNamesAndTypes" || params.expected_sql_type != SQL_TYPE_DATE) { SQL_TIMESTAMP_STRUCT col = {}; SQLLEN col_ind = 0; @@ -155,7 +148,7 @@ TEST_P(DateTime, GetData) { &col_ind )); - EXPECT_EQ(col, expected_timestamp_val); + EXPECT_EQ(col, params.expected_timestamp_val); } } @@ -177,59 +170,59 @@ INSTANTIATE_TEST_SUITE_P( MiscellaneousTest, DateTime, ::testing::Values( - std::make_tuple("Date", "ODBCDriver2", "Europe/Moscow", + DateTimeParams{"Date", "ODBCDriver2", "Europe/Moscow", "toDate('2020-03-25')", SQL_TYPE_DATE, "2020-03-25", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 0, 0, 0, 0} - ), - std::make_tuple("DateTime", "ODBCDriver2", "Europe/Moscow", + }, + DateTimeParams{"DateTime", "ODBCDriver2", "Europe/Moscow", "toDateTime('2020-03-25 12:11:22')", SQL_TYPE_TIMESTAMP, "2020-03-25 12:11:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 0} - ), - std::make_tuple("DateTime_TZ", "ODBCDriver2", "Europe/Moscow", + }, + DateTimeParams{"DateTime_TZ", "ODBCDriver2", "Europe/Moscow", "toDateTime('2020-03-25 12:11:22', 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, "2020-03-25 12:11:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 0} - ), - std::make_tuple("DateTime64_0", "ODBCDriver2", "Europe/Moscow", + }, + DateTimeParams{"DateTime64_0", "ODBCDriver2", "Europe/Moscow", "toDateTime64('2020-03-25 12:11:22.123456789', 0)", SQL_TYPE_TIMESTAMP, "2020-03-25 12:11:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 0} - ), - std::make_tuple("DateTime64_4", "ODBCDriver2", "Europe/Moscow", + }, + DateTimeParams{"DateTime64_4", "ODBCDriver2", "Europe/Moscow", "toDateTime64('2020-03-25 12:11:22.123456789', 4)", SQL_TYPE_TIMESTAMP, "2020-03-25 12:11:22.1234", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 123400000} - ), - std::make_tuple("DateTime64_9", "ODBCDriver2", "Europe/Moscow", + }, + DateTimeParams{"DateTime64_9", "ODBCDriver2", "Europe/Moscow", "toDateTime64('2020-03-25 12:11:22.123456789', 9)", SQL_TYPE_TIMESTAMP, "2020-03-25 12:11:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 123456789} - ), - std::make_tuple("DateTime64_9_TZ", "ODBCDriver2", "Europe/Moscow", + }, + DateTimeParams{"DateTime64_9_TZ", "ODBCDriver2", "Europe/Moscow", "toDateTime64('2020-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, "2020-03-25 12:11:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 12, 11, 22, 123456789} - ), + }, // TODO: remove this once the formats behave identically. - std::make_tuple("Date", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + DateTimeParams{"Date", "RowBinaryWithNamesAndTypes", "Europe/Moscow", "toDate('2020-03-25')", SQL_TYPE_DATE, "2020-03-25", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 0, 0, 0, 0} - ), - std::make_tuple("DateTime_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + }, + DateTimeParams{"DateTime_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", "toDateTime('2020-03-25 12:11:22', 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, "2020-03-25 09:26:22", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 9, 26, 22, 0} - ), - std::make_tuple("DateTime64_9_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + }, + DateTimeParams{"DateTime64_9_TZ", "RowBinaryWithNamesAndTypes", "Europe/Moscow", "toDateTime64('2020-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, "2020-03-25 09:26:22.123456789", SQL_TIMESTAMP_STRUCT{2020, 3, 25, 9, 26, 22, 123456789} - )/*, + }/*, // TODO: uncomment once the target ClickHouse server is 21.4+ - std::make_tuple("DateTime64_9_TZ_pre_epoch", "RowBinaryWithNamesAndTypes", "Europe/Moscow", + DateTimeParams{"DateTime64_9_TZ_pre_epoch", "RowBinaryWithNamesAndTypes", "Europe/Moscow", "toDateTime64('1955-03-25 12:11:22.123456789', 9, 'Asia/Kathmandu')", SQL_TYPE_TIMESTAMP, "1955-03-25 09:26:22.123456789", SQL_TIMESTAMP_STRUCT{1955, 3, 25, 9, 26, 22, 123456789} - ) + } */ ), [] (const auto & param_info) { - return std::get<0>(param_info.param) + "_over_" + std::get<1>(param_info.param); + return param_info.param.name + "_over_" + param_info.param.format; } ); From 045ff6e595f14013bfa75905b1098cf682f2d5a9 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Tue, 3 Aug 2021 17:23:16 +0400 Subject: [PATCH 19/20] Bump submodules --- contrib/folly | 2 +- contrib/googletest | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/folly b/contrib/folly index 4548ec7bf..451819f6c 160000 --- a/contrib/folly +++ b/contrib/folly @@ -1 +1 @@ -Subproject commit 4548ec7bfd2753a1f4b63a605486a83bc72bc9c9 +Subproject commit 451819f6cb921f3ad2d69dbbf75afacfe74a3f49 diff --git a/contrib/googletest b/contrib/googletest index 8d51ffdfa..2d924d7a9 160000 --- a/contrib/googletest +++ b/contrib/googletest @@ -1 +1 @@ -Subproject commit 8d51ffdfab10b3fba636ae69bc03da4b54f8c235 +Subproject commit 2d924d7a971e9667d76ad09727fb2402b4f8a1e3 From ce0b113af833cbcf3e9b113e4d9f087f81fa0a6a Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Wed, 4 Aug 2021 00:43:07 +0400 Subject: [PATCH 20/20] Fix iODBC tests - iODBC doesn't recognize '{' and '}'-enclosed connection string parameter values --- driver/test/misc_it.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver/test/misc_it.cpp b/driver/test/misc_it.cpp index 1979a188c..2263b8046 100755 --- a/driver/test/misc_it.cpp +++ b/driver/test/misc_it.cpp @@ -168,7 +168,7 @@ TEST_P(ConnectionFailureReporing, TryQuery) { { const auto & dsn_orig = TestEnvironment::getInstance().getDSN(); - std::string cs_orig = "DSN={" + dsn_orig + "};" + cs_extras; + std::string cs_orig = "DSN=" + dsn_orig + ";" + cs_extras; const auto cs = fromUTF8(cs_orig); auto * cs_wptr = const_cast(cs.c_str()); @@ -271,7 +271,7 @@ TEST_P(HugeIntTypeReporting, Check) { const auto & [/* unused */name, cs_extras, expected_sql_type] = params; const auto & dsn = TestEnvironment::getInstance().getDSN(); - const auto cs = "DSN={" + dsn + "};" + cs_extras; + const auto cs = "DSN=" + dsn + ";" + cs_extras; connect(cs); const auto query_orig = "SELECT CAST('0', '" + type + "') AS col";