From 26dfbaebead6bccefeb03fb5ed3afce24985cf97 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Mon, 23 Dec 2024 21:39:26 +0100 Subject: [PATCH] [Github CI] Enable Oracle database tests Signed-off-by: Christian Parpart --- .github/prepare-test-run.sh | 21 +++-- .github/workflows/build.yml | 28 ++++--- .vimspector.json | 2 +- docs/best-practices.md | 7 ++ src/Lightweight/CMakeLists.txt | 1 + src/Lightweight/DataBinder/Primitives.cpp | 83 +++++++++++++++++++ src/Lightweight/DataBinder/Primitives.hpp | 34 +++++++- .../QueryFormatter/OracleFormatter.hpp | 9 +- src/tests/CoreTests.cpp | 6 +- src/tests/DataBinderTests.cpp | 20 +++-- src/tests/DataMapperTests.cpp | 6 +- src/tests/Utils.hpp | 59 +++++++++---- 12 files changed, 214 insertions(+), 62 deletions(-) create mode 100644 src/Lightweight/DataBinder/Primitives.cpp diff --git a/.github/prepare-test-run.sh b/.github/prepare-test-run.sh index ac622ccc..d46a7c4d 100755 --- a/.github/prepare-test-run.sh +++ b/.github/prepare-test-run.sh @@ -144,6 +144,12 @@ setup_oracle() { local DB_USER=$USER local target_dir="$HOME/oracle" + # Override DB_NAME to FREEPDB1, as this is already created in the Oracle Docker image + # and it helps avoiding unnecessary database creation time. + DB_NAME="FREEPDB1" + + set -ex + # References # - https://github.com/gvenzl/oci-oracle-free @@ -151,7 +157,6 @@ setup_oracle() { docker pull gvenzl/oracle-free:$ORACLE_VERSION docker run -d -p 1521:1521 \ -e ORACLE_PASSWORD="$DB_PASSWORD" \ - -e ORACLE_DATABASE="$DB_NAME" \ -e APP_USER="$DB_USER" \ -e APP_USER_PASSWORD="$DB_PASSWORD" \ gvenzl/oracle-free:$ORACLE_VERSION @@ -176,8 +181,8 @@ setup_oracle() { sudo cp -v etc/odbcinst.ini /etc/ sudo apt install -y libaio-dev - sudo ln -s /usr/lib/x86_64-linux-gnu/libaio.so.1t64 /usr/local/lib/libaio.so.1 - ldconfig + sudo ln -s /usr/lib/x86_64-linux-gnu/libaio.so.1t64 /usr/lib/x86_64-linux-gnu/libaio.so.1 + sudo ldconfig sudo odbcinst -q -d sudo odbcinst -q -d -n "Oracle ${oracle_odbc_ver_major} ODBC driver" @@ -185,14 +190,14 @@ setup_oracle() { # test connection with: export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:${target_dir}/instantclient_21_3 - ${target_dir}/instantclient_21_3/sqlplus $DB_USER/$DB_PASSWORD@localhost:1521/$DB_NAME <> "${GITHUB_OUTPUT}" - echo "ODBC_CONNECTION_STRING=DRIVER=Oracle ${oracle_odbc_ver_major} ODBC driver;SERVER=localhost;PORT=1521;UID=system;PWD=$DB_PASSWORD;DBA=W" >> "${GITHUB_OUTPUT}" + echo "ODBC_CONNECTION_STRING=DRIVER=Oracle ${oracle_odbc_ver_major} ODBC driver;SERVER=localhost;PORT=1521;UID=$DB_USER;PWD=$DB_PASSWORD;DBQ=$DB_NAME;DBA=W" >> "${GITHUB_OUTPUT}" } setup_mysql() { diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 959e2594..5788fc17 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -245,12 +245,12 @@ jobs: - name: "Setup ${{ matrix.database }}" id: setup run: bash ./.github/prepare-test-run.sh "${{ matrix.database }}" - # Don't do ODBC tracing for now - # - name: "Enable ODBC tracing" - # run: | - # echo "[ODBC]" | sudo tee -a /etc/odbcinst.ini - # echo "Trace=Yes" | sudo tee -a /etc/odbcinst.ini - # echo "TraceFile=/dev/stdout" | sudo tee -a /etc/odbcinst.ini + - name: "Enable ODBC tracing" + if: ${{ matrix.database == 'Oracle' }} + run: | + echo "[ODBC]" | sudo tee -a /etc/odbcinst.ini + echo "Trace=Yes" | sudo tee -a /etc/odbcinst.ini + echo "TraceFile=/dev/stdout" | sudo tee -a /etc/odbcinst.ini - name: "~/.odbc.ini: set ServerName" if: ${{ matrix.database == 'Oracle' }} run: | @@ -260,15 +260,19 @@ jobs: run: | echo "ODBC_CONNECTION_STRING=${{ steps.setup.outputs.ODBC_CONNECTION_STRING }}" ldd /home/runner/oracle/instantclient_21_3/libsqora.so.21.1 + - name: "Inspect dependencies" + run: ldd /usr/local/bin/LightweightTest - name: "Run SQL tests" env: ODBC_CONNECTION_STRING: "${{ steps.setup.outputs.ODBC_CONNECTION_STRING }}" run: | - # Don't do --error-exitcode=1 for now - valgrind \ - --leak-check=full \ - --leak-resolution=high \ - --num-callers=64 \ - LightweightTest + set -ex + if [[ "${{ matrix.database }}" = "SQLite3" ]]; then + CMD_PREFIX="valgrind --leak-check=full --leak-resolution=high --num-callers=64 --error-exitcode=1" + elif [[ "${{ matrix.database }}" = "Oracle" ]]; then + export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:~/oracle/instantclient_21_3 + CMD_PREFIX="strace -f" + fi + $CMD_PREFIX LightweightTest # }}} diff --git a/.vimspector.json b/.vimspector.json index d7aee28d..0874e32b 100644 --- a/.vimspector.json +++ b/.vimspector.json @@ -55,7 +55,7 @@ "environment": [ { "name": "ODBC_CONNECTION_STRING", - "value": "DRIVER=Oracle 21 ODBC driver;SERVER=localhost;PORT=1521;UID=system;PWD=Super1Secret.;DBA=W" + "value": "DRIVER=Oracle 21 ODBC driver;SERVER=localhost;PORT=1521;UID=system;PWD=Super1Secret.;DBA=W;Database=FREEPDB1" } ], "cwd": "${workspaceRoot}", diff --git a/docs/best-practices.md b/docs/best-practices.md index 04a1ff81..2754e5c8 100644 --- a/docs/best-practices.md +++ b/docs/best-practices.md @@ -50,3 +50,10 @@ of parsing, analyzing, and compiling the SQL queries. When querying large result sets, use pagination to limit the number of results returned in a single response. This will help to reduce the response time and the load on the server. + +## SQL server variation challenges + +### 64-bit integer handling in Oracle database + +Oracle database does not support 64-bit integers natively. +When working with 64-bit integers in Oracle database, you need to use the `SqlNumeric` column types. diff --git a/src/Lightweight/CMakeLists.txt b/src/Lightweight/CMakeLists.txt index 38d36904..0753dd1e 100644 --- a/src/Lightweight/CMakeLists.txt +++ b/src/Lightweight/CMakeLists.txt @@ -67,6 +67,7 @@ set(HEADER_FILES ) set(SOURCE_FILES + DataBinder/Primitives.cpp DataBinder/SqlGuid.cpp DataBinder/SqlVariant.cpp DataBinder/UnicodeConverter.cpp diff --git a/src/Lightweight/DataBinder/Primitives.cpp b/src/Lightweight/DataBinder/Primitives.cpp new file mode 100644 index 00000000..76204a11 --- /dev/null +++ b/src/Lightweight/DataBinder/Primitives.cpp @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include "BasicStringBinder.hpp" +#include "Primitives.hpp" +#include "SqlFixedString.hpp" + +template +SQLRETURN Int64DataBinderHelper::InputParameter(SQLHSTMT stmt, + SQLUSMALLINT column, + Int64Type const& value, + SqlDataBinderCallback& cb) noexcept +{ + if (cb.ServerType() == SqlServerType::ORACLE) + { + using StringType = SqlFixedString<21>; + auto strValue = std::make_shared(); + std::to_chars(strValue->data(), strValue->data() + strValue->capacity(), value, 10); + cb.PlanPostExecuteCallback([strValue]() {}); // Defer the destruction of the string until after the execute + return SqlDataBinder::InputParameter(stmt, column, *strValue, cb); + } + else + { + return SqlSimpleDataBinder:: + InputParameter(stmt, column, value, cb); + } +} + +template +SQLRETURN Int64DataBinderHelper::OutputColumn( + SQLHSTMT stmt, SQLUSMALLINT column, Int64Type* result, SQLLEN* indicator, SqlDataBinderCallback& cb) noexcept +{ + if (cb.ServerType() == SqlServerType::ORACLE) + { + using StringType = SqlFixedString<21>; + auto buffer = std::make_shared(); + auto const sqlResult = SqlDataBinder::OutputColumn(stmt, column, buffer.get(), indicator, cb); + if (SQL_SUCCEEDED(sqlResult)) + { + cb.PlanPostProcessOutputColumn([buffer, result, indicator]() { + if (*indicator != SQL_NULL_DATA && *indicator != SQL_NO_TOTAL) + { + std::from_chars(buffer->data(), buffer->data() + buffer->size(), *result, 10); + } + }); + } + return sqlResult; + } + else + { + return SqlSimpleDataBinder::OutputColumn( + stmt, column, result, indicator, cb); + } +} + +template +SQLRETURN Int64DataBinderHelper::GetColumn( + SQLHSTMT stmt, SQLUSMALLINT column, Int64Type* result, SQLLEN* indicator, SqlDataBinderCallback const& cb) noexcept +{ + if (cb.ServerType() == SqlServerType::ORACLE) + { + using StringType = SqlFixedString<21>; + auto buffer = StringType {}; + auto const sqlResult = SqlDataBinder::GetColumn(stmt, column, &buffer, indicator, cb); + if (SQL_SUCCEEDED(sqlResult) && *indicator != SQL_NULL_DATA && *indicator != SQL_NO_TOTAL) + { + std::from_chars(buffer.data(), buffer.data() + buffer.size(), *result, 10); + } + return sqlResult; + } + else + { + return SqlSimpleDataBinder::GetColumn( + stmt, column, result, indicator, cb); + } +} + +template struct Int64DataBinderHelper; +template struct Int64DataBinderHelper; + +#if !defined(_WIN32) && !defined(__APPLE__) +template struct Int64DataBinderHelper; +template struct Int64DataBinderHelper; +#endif diff --git a/src/Lightweight/DataBinder/Primitives.hpp b/src/Lightweight/DataBinder/Primitives.hpp index 01739f32..1f375ca6 100644 --- a/src/Lightweight/DataBinder/Primitives.hpp +++ b/src/Lightweight/DataBinder/Primitives.hpp @@ -37,6 +37,31 @@ struct SqlSimpleDataBinder } }; +template +struct LIGHTWEIGHT_API Int64DataBinderHelper +{ + static constexpr SqlColumnTypeDefinition ColumnType = SqlColumnTypeDefinitions::Bigint {}; + + static SQLRETURN InputParameter(SQLHSTMT stmt, + SQLUSMALLINT column, + Int64Type const& value, + SqlDataBinderCallback& cb) noexcept; + + static SQLRETURN OutputColumn( + SQLHSTMT stmt, SQLUSMALLINT column, Int64Type* result, SQLLEN* indicator, SqlDataBinderCallback& cb) noexcept; + + static SQLRETURN GetColumn(SQLHSTMT stmt, + SQLUSMALLINT column, + Int64Type* result, + SQLLEN* indicator, + SqlDataBinderCallback const& cb) noexcept; + + static LIGHTWEIGHT_FORCE_INLINE std::string Inspect(Int64Type value) + { + return std::to_string(value); + } +}; + // clang-format off template <> struct SqlDataBinder: SqlSimpleDataBinder {}; template <> struct SqlDataBinder: SqlSimpleDataBinder {}; @@ -44,13 +69,14 @@ template <> struct SqlDataBinder: SqlSimpleDataBinder struct SqlDataBinder: SqlSimpleDataBinder {}; template <> struct SqlDataBinder: SqlSimpleDataBinder {}; template <> struct SqlDataBinder: SqlSimpleDataBinder {}; -template <> struct SqlDataBinder: SqlSimpleDataBinder {}; -template <> struct SqlDataBinder: SqlSimpleDataBinder {}; +template <> struct SqlDataBinder: Int64DataBinderHelper {}; +template <> struct SqlDataBinder: Int64DataBinderHelper {}; +//template <> struct SqlDataBinder: Int64DataBinderHelper {}; template <> struct SqlDataBinder: SqlSimpleDataBinder {}; template <> struct SqlDataBinder: SqlSimpleDataBinder {}; #if !defined(_WIN32) && !defined(__APPLE__) -template <> struct SqlDataBinder: SqlSimpleDataBinder {}; -template <> struct SqlDataBinder: SqlSimpleDataBinder {}; +template <> struct SqlDataBinder: Int64DataBinderHelper {}; +template <> struct SqlDataBinder: Int64DataBinderHelper {}; #endif #if defined(__APPLE__) // size_t is a different type on macOS template <> struct SqlDataBinder: SqlSimpleDataBinder {}; diff --git a/src/Lightweight/QueryFormatter/OracleFormatter.hpp b/src/Lightweight/QueryFormatter/OracleFormatter.hpp index 711021d1..4596062a 100644 --- a/src/Lightweight/QueryFormatter/OracleFormatter.hpp +++ b/src/Lightweight/QueryFormatter/OracleFormatter.hpp @@ -79,13 +79,8 @@ class OracleSqlQueryFormatter final: public SQLiteQueryFormatter using namespace SqlColumnTypeDefinitions; return std::visit( detail::overloaded { - [](Bigint const&) -> std::string { return "NUMBER(19, 0)"; }, - [](Binary const& type) -> std::string { - if (type.size) - return std::format("BLOB({})", type.size); - else - return "BLOB"; - }, + [](Bigint const&) -> std::string { return "NUMBER(21, 0)"; }, + [](Binary const&) -> std::string { return "BLOB"; }, [](Bool const&) -> std::string { return "BIT"; }, [](Char const& type) -> std::string { return std::format("CHAR({})", type.size); }, [](Date const&) -> std::string { return "DATE"; }, diff --git a/src/tests/CoreTests.cpp b/src/tests/CoreTests.cpp index 2d82c643..1d575485 100644 --- a/src/tests/CoreTests.cpp +++ b/src/tests/CoreTests.cpp @@ -325,6 +325,8 @@ TEST_CASE_METHOD(SqlTestFixture, "LastInsertId", "[SqlStatement]") { auto stmt = SqlStatement {}; + UNSUPPORTED_DATABASE(stmt, SqlServerType::ORACLE); + CreateEmployeesTable(stmt); FillEmployeesTable(stmt); @@ -396,14 +398,14 @@ TEST_CASE_METHOD(SqlTestFixture, "Prepare and move", "[SqlStatement]") struct Simple1 { - uint64_t pk; + uint32_t pk; SqlAnsiString<30> c1; SqlAnsiString<30> c2; }; struct Simple2 { - uint64_t pk; + uint32_t pk; SqlAnsiString<30> c1; SqlAnsiString<30> c2; }; diff --git a/src/tests/DataBinderTests.cpp b/src/tests/DataBinderTests.cpp index a3da7768..f7948f74 100644 --- a/src/tests/DataBinderTests.cpp +++ b/src/tests/DataBinderTests.cpp @@ -443,13 +443,17 @@ struct TestTypeTraits template <> struct TestTypeTraits { - static constexpr auto blacklist = std::array { - std::pair { SqlServerType::ORACLE, "Oracle does not support 64-bit integers via SQL_BIGINT it seems"sv }, - }; static constexpr auto inputValue = (std::numeric_limits::max)(); static constexpr auto expectedOutputValue = (std::numeric_limits::max)(); }; +template <> +struct TestTypeTraits +{ + static constexpr auto inputValue = (std::numeric_limits::max)(); + static constexpr auto expectedOutputValue = (std::numeric_limits::max)(); +}; + template <> struct TestTypeTraits { @@ -654,6 +658,9 @@ struct TestTypeTraits template <> struct TestTypeTraits { + static constexpr auto blacklist = std::array { + std::pair { SqlServerType::ORACLE, "TODO: Oracle"sv }, + }; static constexpr auto sqlColumnTypeNameOverride = SqlColumnTypeDefinitions::Binary { 50 }; static auto const inline inputValue = SqlBinary { 0x00, 0x02, 0x03, 0x00, 0x05 }; static auto const inline expectedOutputValue = SqlBinary { 0x00, 0x02, 0x03, 0x00, 0x05 }; @@ -678,6 +685,7 @@ using TypesToTest = std::tuple< int16_t, int32_t, int64_t, + // TODO: uint64_t, std::string, std::string_view, std::u16string, @@ -763,8 +771,7 @@ TEMPLATE_LIST_TEST_CASE("SqlDataBinder specializations", "[SqlDataBinder]", Type // { // stmt.ExecuteDirect("SELECT Value FROM Test"); // auto actualValue = [&]() -> TestType { - // if constexpr (requires(SqlServerType st) { TestTypeTraits::outputInitializer(st); - // }) + // if constexpr (requires(SqlServerType st) { TestTypeTraits::outputInitializer(st); }) // return TestTypeTraits::outputInitializer(conn.ServerType()); // else if constexpr (requires { TestTypeTraits::outputInitializer; }) // return TestTypeTraits::outputInitializer; @@ -776,8 +783,7 @@ TEMPLATE_LIST_TEST_CASE("SqlDataBinder specializations", "[SqlDataBinder]", Type // if constexpr (std::is_convertible_v && !std::integral) // CHECK_THAT( // double(actualValue), - // (Catch::Matchers::WithinAbs(double(TestTypeTraits::expectedOutputValue), - // 0.001))); + // (Catch::Matchers::WithinAbs(double(TestTypeTraits::expectedOutputValue), 0.001))); // else // CHECK(actualValue == TestTypeTraits::expectedOutputValue); // } diff --git a/src/tests/DataMapperTests.cpp b/src/tests/DataMapperTests.cpp index c599f2b3..0ea0a541 100644 --- a/src/tests/DataMapperTests.cpp +++ b/src/tests/DataMapperTests.cpp @@ -728,10 +728,10 @@ TEST_CASE_METHOD(SqlTestFixture, "Table with aliased column names", "[DataMapper struct PersonDifferenceView { - Field id; - Field> name; + Field id {}; + Field> name {}; Field is_active { true }; - Field age; + Field age {}; }; TEST_CASE_METHOD(SqlTestFixture, "Test DifferenceView", "[DataMapper]") diff --git a/src/tests/Utils.hpp b/src/tests/Utils.hpp index ff0b8c43..85f7afc7 100644 --- a/src/tests/Utils.hpp +++ b/src/tests/Utils.hpp @@ -246,7 +246,7 @@ constexpr void FixedPointIterate(Getter const& getter, Callable const& callable) class SqlTestFixture { public: - static inline std::string_view const testDatabaseName = "LightweightTest"; + static inline std::string testDatabaseName = "LightweightTest"; static inline bool odbcTrace = false; using MainProgramArgs = std::tuple; @@ -349,6 +349,16 @@ class SqlTestFixture { auto stmt = SqlStatement(); REQUIRE(stmt.IsAlive()); + + // On Github CI, we use the pre-created database "FREEPDB1" for Oracle + char dbName[100]; // Buffer to store the database name + SQLSMALLINT dbNameLen {}; + SQLGetInfo(stmt.Connection().NativeHandle(), SQL_DATABASE_NAME, dbName, sizeof(dbName), &dbNameLen); + if (dbNameLen > 0) + testDatabaseName = dbName; + else if (stmt.Connection().ServerType() == SqlServerType::ORACLE) + testDatabaseName = "FREEPDB1"; + DropAllTablesInDatabase(stmt); } @@ -383,6 +393,7 @@ class SqlTestFixture stmt.ExecuteDirect(std::format("USE \"{}\"", testDatabaseName)); [[fallthrough]]; case SqlServerType::SQLITE: + case SqlServerType::ORACLE: case SqlServerType::UNKNOWN: { auto const tableNames = GetAllTableNames(stmt); for (auto const& tableName: tableNames) @@ -400,30 +411,43 @@ class SqlTestFixture for (auto& createdTable: std::views::reverse(m_createdTables)) stmt.ExecuteDirect(std::format("DROP TABLE IF EXISTS \"{}\" CASCADE", createdTable)); break; - case SqlServerType::ORACLE: { - // Drop user-created tables - stmt.ExecuteDirect(R"SQL( - SELECT user_tables.table_name FROM user_tables - LEFT JOIN sys.user_objects ON user_objects.object_type = 'TABLE' AND user_objects.object_name = user_tables.table_name - WHERE user_objects.oracle_maintained != 'Y' - )SQL"); - std::vector tableNames; - while (stmt.FetchRow()) - tableNames.emplace_back(stmt.GetColumn(1)); - for (auto const& tableName: tableNames) - stmt.ExecuteDirect(std::format("DROP TABLE \"{}\"", tableName)); - break; - } } m_createdTables.clear(); } private: + static std::vector GetAllTableNamesForOracle(SqlStatement& stmt) + { + auto result = std::vector {}; + stmt.Prepare(R"SQL(SELECT table_name + FROM user_tables + WHERE table_name NOT LIKE '%$%' + AND table_name NOT IN ('SCHEDULER_JOB_ARGS_TBL', 'SCHEDULER_PROGRAM_ARGS_TBL', 'SQLPLUS_PRODUCT_PROFILE') + ORDER BY table_name)SQL"); + stmt.Execute(); + while (stmt.FetchRow()) + { + result.emplace_back(stmt.GetColumn(1)); + } + return result; + } + static std::vector GetAllTableNames(SqlStatement& stmt) { - using namespace std::string_view_literals; + if (stmt.Connection().ServerType() == SqlServerType::ORACLE) + return GetAllTableNamesForOracle(stmt); + + using namespace std::string_literals; auto result = std::vector(); - auto const schemaName = stmt.Connection().ServerType() == SqlServerType::MICROSOFT_SQL ? "dbo"sv : ""sv; + auto const schemaName = [&] { + switch (stmt.Connection().ServerType()) + { + case SqlServerType::MICROSOFT_SQL: + return "dbo"s; + default: + return ""s; + } + }(); auto const sqlResult = SQLTables(stmt.NativeHandle(), (SQLCHAR*) testDatabaseName.data(), (SQLSMALLINT) testDatabaseName.size(), @@ -578,4 +602,3 @@ inline void FillEmployeesTable(SqlStatement& stmt) stmt.Execute("Bob", "Johnson", 60'000); stmt.Execute("Charlie", "Brown", 70'000); } -