From 2be39e1ad820b24ce0f1592a592b1de9efff2bfe Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 1 Jul 2026 16:25:47 +0200 Subject: [PATCH 1/4] Add RowArrayCursor fixed-width prefetch tests and Codecov CI job CHAR(N)/NCHAR(N) columns are excluded from the transparent automatic prefetch gate and always fall back to per-row SQLGetData there, but RowArrayCursor (the underlying bulk block-fetch mechanism reachable via ExecuteBatchFetch) binds and reads them directly. Add coverage for that path: right-padding, NULL vs empty-string, a value that exactly fills its declared width, and a single-block (no-continuation) fetch. Also add a "coverage" CI job mirroring the existing "Sanitizers" job's structure (clang-coverage preset, sqlite3-only, no Docker services) and upload the resulting lcov report to Codecov via codecov-action@v5. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yml | 49 +++++ src/tests/SqlStatementPrefetchTests.cpp | 274 ++++++++++++++++++++++++ 2 files changed, 323 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 92dc7f88e..d276e6d0a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -925,3 +925,52 @@ jobs: ./out/build/clang-${{ matrix.sanitizer }}/src/tests/LightweightTest --test-env=sqlite3 # }}} + # {{{ Coverage + coverage: + name: "Code coverage" + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v6 + - name: Fetch tags + run: git fetch --prune --unshallow --tags + - name: ccache + uses: hendrikmuhs/ccache-action@v1.2 + with: + key: "ccache-ubuntu2404-clang-coverage" + max-size: 256M + - name: "update APT database" + run: sudo apt -q update + - name: Install Clang + run: | + wget https://apt.llvm.org/llvm.sh + chmod +x llvm.sh + sudo ./llvm.sh ${{ env.CLANG_TOOLS_VERSION }} + - name: "install dependencies" + # Rename the libsqliteodbc driver section to match the Windows driver + # name so the shared .test-env.yml works on both platforms. + run: | + sudo apt install -y cmake ninja-build catch2 unixodbc-dev sqlite3 libsqlite3-dev libsqliteodbc uuid-dev libyaml-cpp-dev libzip-dev lcov + sudo sed -i 's/^\[SQLite3\]$/[SQLite3 ODBC Driver]/' /etc/odbcinst.ini + - name: "cmake" + run: | + cmake --preset clang-coverage \ + -D CMAKE_CXX_COMPILER=clang++-${{ env.CLANG_TOOLS_VERSION }} \ + -D CMAKE_C_COMPILER=clang-${{ env.CLANG_TOOLS_VERSION }} + - name: "build" + run: cmake --build --preset clang-coverage -- -j$(nproc) + # The `coverage` target (cmake/Coverage.cmake) runs the suite against sqlite3/postgres/mssql + # (the latter two tolerate connection failure via `|| true`) and captures lcov data. Only + # sqlite3 is reachable in this job (no Docker services here), matching the "Sanitizers" job's + # scope — the full 3-database matrix is exercised, uninstrumented, by dbms_test_matrix; this + # job's purpose is a coverage percentage, not additional correctness coverage. + - name: "Run tests and generate lcov report" + run: cmake --build --preset clang-coverage --target coverage + - name: "Upload coverage to Codecov" + uses: codecov/codecov-action@v5 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: ./out/build/clang-coverage/coverage/coverage.info + fail_ci_if_error: false + verbose: true + + # }}} diff --git a/src/tests/SqlStatementPrefetchTests.cpp b/src/tests/SqlStatementPrefetchTests.cpp index 2452bc136..c1ee9011b 100644 --- a/src/tests/SqlStatementPrefetchTests.cpp +++ b/src/tests/SqlStatementPrefetchTests.cpp @@ -122,6 +122,17 @@ SqlStatement MakePerRowStatement() return stmt; } +// Strips the server's CHAR(N)/NCHAR(N) right-padding (trailing ASCII spaces) so a fixed-width +// column's value compares equal regardless of how much padding the driver returned it with. +std::optional TrimRight(std::optional value) +{ + if (!value.has_value()) + return std::nullopt; + while (!value->empty() && value->back() == ' ') + value->pop_back(); + return value; +} + } // namespace TEST_CASE_METHOD(SqlTestFixture, "Prefetch: raw GetColumn loop collapses round-trips", "[prefetch]") @@ -544,6 +555,269 @@ TEST_CASE_METHOD(SqlTestFixture, "Prefetch: GUID column round-trips", "[prefetch REQUIRE(actual.size() == rowCount); } +TEST_CASE_METHOD(SqlTestFixture, "Prefetch: RowArrayCursor bulk-reads fixed-width CHAR/NCHAR columns", "[prefetch]") +{ + // Fixed-width text (CHAR(N)/NCHAR(N)) is excluded from the transparent automatic prefetch gate + // (PrefetchableSqlType in SqlStatement.cpp) and always falls back to per-row SQLGetData there. + // RowArrayCursor — the actual bulk block-fetch mechanism reachable via ExecuteBatchFetch — binds + // and reads fixed-width text directly, independent of that gate. This pins that a bulk fetch of + // CHAR(N)/NCHAR(N) columns, including NULLs and the server's right-padding, is byte-identical to + // the trusted per-row reference. + auto stmt = SqlStatement {}; + + stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) { + migration.CreateTable("PrefetchFixedWidth") + .PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {}) + .Column("Code", SqlColumnTypeDefinitions::Char { 8 }) // narrow fixed-width, right-padded + .Column("WideCode", SqlColumnTypeDefinitions::NChar { 8 }); // wide fixed-width, right-padded + }); + + constexpr std::size_t rowCount = TestPrefetchDepth + 6; + stmt.Prepare(stmt.Query("PrefetchFixedWidth") + .Insert() + .Set("Id", SqlWildcard) + .Set("Code", SqlWildcard) + .Set("WideCode", SqlWildcard)); + for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1)) + { + // Every 4th row is NULL in both fixed-width columns; the rest carry a short value shorter + // than the column's declared width, so the server's right-padding is actually exercised. + if (i % 4 == 0) + (void) stmt.Execute(static_cast(i), std::optional {}, std::optional {}); + else + (void) stmt.Execute(static_cast(i), + std::optional { std::format("C{}", i % 100) }, + std::optional { std::format("W{}", i % 100) }); + } + + // Trusted reference: the classic per-row SQLGetData path. SqlTrimmedFixedString strips the + // server's CHAR(N) right-padding on the narrow column; the wide column is read as plain + // std::string (already-proven UTF-16 -> UTF-8 conversion) and trimmed manually here to mirror + // RowArrayCursor::GetString's own trailing-space trim below, since SqlTrimmedWideFixedString's + // GetColumn does not currently apply its trim (a separate, pre-existing gap outside this test's + // scope — see SqlDataBinder::GetColumn in BasicStringBinder.hpp). + using Row = std::tuple, std::optional>; + auto readPerRow = [&]() { + std::vector rows; + auto reference = MakePerRowStatement(); + auto cursor = + reference.ExecuteDirect(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedWidth" ORDER BY "Id")"sv); + while (cursor.FetchRow()) + { + auto const id = cursor.GetColumn(1); + auto const code = cursor.GetNullableColumn>(2); + auto const wideCode = TrimRight(cursor.GetNullableColumn(3)); + rows.emplace_back( + id, code.has_value() ? std::optional { code->ToStringView() } : std::nullopt, wideCode); + } + return rows; + }; + auto const expected = readPerRow(); + REQUIRE(expected.size() == rowCount); + + // RowArrayCursor directly — the bulk block-fetch mechanism under test. + std::vector actual; + { + auto cursor = stmt.ExecuteBatchFetch(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedWidth" ORDER BY "Id")"sv, + TestPrefetchDepth); + + // Confirm the columns actually bound as text (Char/WChar), not silently as something else. + // Some drivers (e.g. SQLite3's ODBC driver, psqlODBC) report narrow CHAR columns as WChar + // regardless of the declared SQL type, so accept either for the narrow column. + auto const isTextlike = [](RowArrayCursor::BoundType type) { + return type == RowArrayCursor::BoundType::Char || type == RowArrayCursor::BoundType::WChar; + }; + CHECK(isTextlike(cursor.ColumnBoundType(2))); + CHECK(cursor.ColumnBoundType(3) == RowArrayCursor::BoundType::WChar); + + std::size_t blocks = 0; + for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray()) + { + ++blocks; + for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock)) + { + auto const id = cursor.GetI64(row, 1).value(); + // Trim the CHAR(8)/NCHAR(8) driver padding to compare against the trimmed reference. + auto code = TrimRight(cursor.GetString(row, 2)); + auto wideCode = TrimRight(cursor.GetString(row, 3)); + actual.emplace_back(id, std::move(code), std::move(wideCode)); + } + } + CHECK(blocks >= 2); // crossed at least one block boundary — genuinely block-fetched, not one row + } + + REQUIRE(actual.size() == rowCount); + CHECK(actual == expected); // byte-identical to the trusted per-row path, including NULLs and padding +} + +TEST_CASE_METHOD(SqlTestFixture, + "Prefetch: RowArrayCursor bulk-reads a fixed-width value that fills its full capacity", + "[prefetch]") +{ + // Regression sibling to #485 (BasicStringBinder::OutputColumn off-by-one on BufferLength) for the + // RowArrayCursor bulk path: a value with NO trailing padding — it fills CHAR(N)/NCHAR(N) exactly — + // must not lose its last character to a buffer sized for the terminator alone. + auto stmt = SqlStatement {}; + + stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) { + migration.CreateTable("PrefetchFixedFull") + .PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {}) + .Column("Code", SqlColumnTypeDefinitions::Char { 8 }) + .Column("WideCode", SqlColumnTypeDefinitions::NChar { 8 }); + }); + + constexpr std::size_t rowCount = TestPrefetchDepth + 6; + stmt.Prepare(stmt.Query("PrefetchFixedFull") + .Insert() + .Set("Id", SqlWildcard) + .Set("Code", SqlWildcard) + .Set("WideCode", SqlWildcard)); + for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1)) + { + // Every value is exactly 8 chars — fills CHAR(8)/NCHAR(8) with no padding at all. + auto const code = std::format("{:08}", i % 100'000'000); + (void) stmt.Execute(static_cast(i), code, code); + } + + auto readPerRow = [&]() { + std::vector> rows; + auto reference = MakePerRowStatement(); + auto cursor = reference.ExecuteDirect(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedFull" ORDER BY "Id")"sv); + while (cursor.FetchRow()) + { + auto const id = cursor.GetColumn(1); + auto const code = cursor.GetColumn>(2); + auto const wideCode = *TrimRight(cursor.GetColumn(3)); + rows.emplace_back(id, std::string { code.ToStringView() }, wideCode); + } + return rows; + }; + auto const expected = readPerRow(); + REQUIRE(expected.size() == rowCount); + + std::vector> actual; + { + auto cursor = stmt.ExecuteBatchFetch(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedFull" ORDER BY "Id")"sv, + TestPrefetchDepth); + for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray()) + for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock)) + actual.emplace_back( + cursor.GetI64(row, 1).value(), cursor.GetString(row, 2).value(), cursor.GetString(row, 3).value()); + } + + REQUIRE(actual.size() == rowCount); + for (std::size_t i = 0; i < rowCount; ++i) + { + CHECK(std::get<0>(actual[i]) == std::get<0>(expected[i])); + CHECK(std::get<1>(actual[i]) == std::get<1>(expected[i])); // full 8 chars, not truncated to 7 + CHECK(std::get<2>(actual[i]) == std::get<2>(expected[i])); + CHECK(std::get<1>(actual[i]).size() == 8); + CHECK(std::get<2>(actual[i]).size() == 8); + } +} + +TEST_CASE_METHOD(SqlTestFixture, + "Prefetch: RowArrayCursor distinguishes NULL from empty string in fixed-width columns", + "[prefetch]") +{ + // NULL and "" are distinct SQL values; a bulk fetch must not collapse one into the other (e.g. by + // treating an all-blank buffer as NULL, or a NULL indicator as an empty-but-non-null string). + auto stmt = SqlStatement {}; + + stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) { + migration.CreateTable("PrefetchFixedEmptyVsNull") + .PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {}) + .Column("Code", SqlColumnTypeDefinitions::Char { 8 }) + .Column("WideCode", SqlColumnTypeDefinitions::NChar { 8 }); + }); + + constexpr std::size_t rowCount = TestPrefetchDepth + 6; + stmt.Prepare(stmt.Query("PrefetchFixedEmptyVsNull") + .Insert() + .Set("Id", SqlWildcard) + .Set("Code", SqlWildcard) + .Set("WideCode", SqlWildcard)); + for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1)) + { + // Every 3rd row is NULL, every other 3rd row is an empty string, the rest carry a short value. + if (i % 3 == 0) + (void) stmt.Execute(static_cast(i), std::optional {}, std::optional {}); + else if (i % 3 == 1) + (void) stmt.Execute(static_cast(i), std::string {}, std::string {}); + else + (void) stmt.Execute(static_cast(i), std::format("C{}", i), std::format("W{}", i)); + } + + auto cursor = stmt.ExecuteBatchFetch( + R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedEmptyVsNull" ORDER BY "Id")"sv, TestPrefetchDepth); + + std::size_t rowsSeen = 0; + for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray()) + { + for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock)) + { + auto const id = cursor.GetI64(row, 1).value(); + ++rowsSeen; + + if (id % 3 == 0) + { + // NULL: the indicator must say so, and GetString must not fabricate a value. + CHECK(cursor.IsCellNull(row, 2)); + CHECK(cursor.IsCellNull(row, 3)); + CHECK_FALSE(cursor.GetString(row, 2).has_value()); + CHECK_FALSE(cursor.GetString(row, 3).has_value()); + } + else if (id % 3 == 1) + { + // Empty string, server-padded to CHAR(8)/NCHAR(8): NOT null, and trims down to "". + CHECK_FALSE(cursor.IsCellNull(row, 2)); + CHECK_FALSE(cursor.IsCellNull(row, 3)); + CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional { "" }); + CHECK(TrimRight(cursor.GetString(row, 3)) == std::optional { "" }); + } + else + { + CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional { std::format("C{}", id) }); + CHECK(TrimRight(cursor.GetString(row, 3)) == std::optional { std::format("W{}", id) }); + } + } + } + CHECK(rowsSeen == rowCount); +} + +TEST_CASE_METHOD(SqlTestFixture, + "Prefetch: RowArrayCursor bulk-reads fixed-width columns within a single block", + "[prefetch]") +{ + // The other fixed-width tests deliberately cross a block boundary; this pins the same correctness + // for a result set that fits in a single FetchArray() call (arrayDepth > rowCount), so the + // single-block code paths (no continuation, no second SQLGetData) are exercised too. + auto stmt = SqlStatement {}; + + stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) { + migration.CreateTable("PrefetchFixedSingleBlock") + .PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {}) + .Column("Code", SqlColumnTypeDefinitions::Char { 8 }); + }); + + constexpr std::size_t rowCount = 5; // well under TestPrefetchDepth + stmt.Prepare(stmt.Query("PrefetchFixedSingleBlock").Insert().Set("Id", SqlWildcard).Set("Code", SqlWildcard)); + for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1)) + (void) stmt.Execute(static_cast(i), std::format("C{}", i)); + + auto cursor = + stmt.ExecuteBatchFetch(R"(SELECT "Id", "Code" FROM "PrefetchFixedSingleBlock" ORDER BY "Id")"sv, TestPrefetchDepth); + + auto const rowsInFirstBlock = cursor.FetchArray(); + REQUIRE(rowsInFirstBlock == rowCount); // the whole result set fit in one block + for (auto const row: std::views::iota(std::size_t { 0 }, rowsInFirstBlock)) + { + auto const id = cursor.GetI64(row, 1).value(); + CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional { std::format("C{}", id) }); + } + CHECK(cursor.FetchArray() == 0); // no more rows +} + // Opt-in micro-benchmark: per-row SQLFetch vs transparent block-prefetch over a large result set. // Run with: LightweightTest "[prefetchbench]" (hidden by the leading '.'). TEST_CASE_METHOD(SqlTestFixture, "Prefetch benchmark", "[.][prefetchbench]") From 6f9bfcd1fb381398959cfec4a7751050ad5e45ca Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 1 Jul 2026 16:39:38 +0200 Subject: [PATCH 2/4] Fix coverage CI: find versioned llvm-cov binary, not bare llvm-cov apt.llvm.org's installer (llvm.sh) only ever installs version-suffixed binaries (llvm-cov-22), never a bare llvm-cov symlink. The unversioned find_program(LLVM_COV_PATH llvm-cov) lookup silently failed on the new coverage CI job, so lcov fell back to the system gcov (from GCC), which cannot parse Clang's gcov-compatible data format: geninfo: ERROR: Incompatible GCC/GCOV version found while processing .../SqlLogger.cpp.gcda Search for the version matching the active compiler first, then a few recent majors, then the unversioned name (covers Homebrew LLVM on macOS, which does install a bare llvm-cov). Co-Authored-By: Claude Sonnet 5 --- cmake/Coverage.cmake | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/cmake/Coverage.cmake b/cmake/Coverage.cmake index 7df17d25f..624c6b26a 100644 --- a/cmake/Coverage.cmake +++ b/cmake/Coverage.cmake @@ -56,7 +56,22 @@ endif() # For Clang, we need to use llvm-cov gcov instead of gcov set(GCOV_TOOL_OPTION "") if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - find_program(LLVM_COV_PATH llvm-cov) + # apt.llvm.org's installer (llvm.sh) only ever installs version-suffixed binaries + # (llvm-cov-22, never a bare llvm-cov symlink), so an unversioned lookup silently + # fails and lcov falls back to the system gcov (from GCC), which cannot parse + # Clang's gcov-compatible data format ("Incompatible GCC/GCOV version" from + # geninfo). Try the version matching the active compiler first, then fall back + # to a few recent majors and finally the unversioned name for other toolchains + # (e.g. Homebrew LLVM on macOS, which does install a bare llvm-cov). + if(CMAKE_CXX_COMPILER_VERSION) + string(REGEX MATCH "^[0-9]+" LLVM_COV_VERSION_SUFFIX "${CMAKE_CXX_COMPILER_VERSION}") + endif() + find_program(LLVM_COV_PATH + NAMES + "llvm-cov-${LLVM_COV_VERSION_SUFFIX}" + llvm-cov-22 llvm-cov-21 llvm-cov-20 llvm-cov-19 llvm-cov-18 + llvm-cov + ) if(LLVM_COV_PATH) # Create a wrapper script for llvm-cov gcov set(GCOV_WRAPPER_SCRIPT "${CMAKE_BINARY_DIR}/llvm-gcov-wrapper.sh") From b1f7ad096341a965b62edb4ec4729300f92508fd Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 1 Jul 2026 17:07:43 +0200 Subject: [PATCH 3/4] Fix flaky bound-type assertion in fixed-width prefetch test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SQLite3's ODBC driver reports CHAR(N)/NCHAR(N) as Char vs WChar inconsistently across platforms (confirmed: WChar for both on Windows, but Char for NCHAR(8) on Linux) — reproduced locally via Docker with the exact CI toolchain (Ubuntu 24.04, Clang 22, clang-asan-ubsan preset) after Sanitizers/macOS jobs failed on this branch while passing on master. No sanitizer defect found; RowArrayCursor normalizes both bound types to UTF-8 correctly either way, so relax the assertion to accept either, matching the pattern already used for the narrow column. Co-Authored-By: Claude Sonnet 5 --- src/tests/SqlStatementPrefetchTests.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/tests/SqlStatementPrefetchTests.cpp b/src/tests/SqlStatementPrefetchTests.cpp index c1ee9011b..b7788e848 100644 --- a/src/tests/SqlStatementPrefetchTests.cpp +++ b/src/tests/SqlStatementPrefetchTests.cpp @@ -622,13 +622,16 @@ TEST_CASE_METHOD(SqlTestFixture, "Prefetch: RowArrayCursor bulk-reads fixed-widt TestPrefetchDepth); // Confirm the columns actually bound as text (Char/WChar), not silently as something else. - // Some drivers (e.g. SQLite3's ODBC driver, psqlODBC) report narrow CHAR columns as WChar - // regardless of the declared SQL type, so accept either for the narrow column. + // Which of Char/WChar a driver picks for a given declared SQL type is driver-specific and does + // not affect correctness (RowArrayCursor::GetString normalizes both to UTF-8) — e.g. SQLite3's + // ODBC driver reports CHAR(N) as WChar on Windows but as Char on Linux, and NCHAR(N) the other + // way around on Linux vs Windows. Accept either for both columns; only the byte-identity check + // below actually matters. auto const isTextlike = [](RowArrayCursor::BoundType type) { return type == RowArrayCursor::BoundType::Char || type == RowArrayCursor::BoundType::WChar; }; CHECK(isTextlike(cursor.ColumnBoundType(2))); - CHECK(cursor.ColumnBoundType(3) == RowArrayCursor::BoundType::WChar); + CHECK(isTextlike(cursor.ColumnBoundType(3))); std::size_t blocks = 0; for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray()) From a0ba94fd75792786a3205cddb9238c64f0cbfe61 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 1 Jul 2026 17:34:21 +0200 Subject: [PATCH 4/4] Fix clang-tidy bugprone-unchecked-optional-access in new prefetch tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace direct cursor.GetI64(...).value() / GetString(...).value() calls with RequireI64/RequireString helpers that REQUIRE + explicit if-throw, mirroring the existing RequireParsed pattern in SqlGuidTests.cpp — the analysis can trace an explicit if/throw as a null check but not a Catch2 REQUIRE macro. Reproduced and verified locally against the exact CI toolchain (Ubuntu 24.04, Clang 22, clang-debug preset + run-clang-tidy) via Docker, since this job only runs on non-master branches. Co-Authored-By: Claude Sonnet 5 --- src/tests/SqlStatementPrefetchTests.cpp | 31 +++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/tests/SqlStatementPrefetchTests.cpp b/src/tests/SqlStatementPrefetchTests.cpp index b7788e848..645300d6a 100644 --- a/src/tests/SqlStatementPrefetchTests.cpp +++ b/src/tests/SqlStatementPrefetchTests.cpp @@ -133,6 +133,29 @@ std::optional TrimRight(std::optional value) return value; } +// Unwraps a RowArrayCursor cell known to be non-NULL (e.g. the primary key column, or a text +// column the test never inserts NULL into), REQUIRE-ing that first. The explicit if-with-throw is +// what clang-tidy's bugprone-unchecked-optional-access analysis recognizes as a null check — +// REQUIRE alone is a macro it cannot reason about (see the same pattern in SqlGuidTests.cpp's +// RequireParsed). +std::int64_t RequireI64(RowArrayCursor const& cursor, std::size_t row, SQLUSMALLINT column) +{ + auto const value = cursor.GetI64(row, column); + REQUIRE(value.has_value()); + if (!value.has_value()) + throw std::runtime_error("REQUIRE failed but flow continued"); // unreachable + return *value; +} + +std::string RequireString(RowArrayCursor const& cursor, std::size_t row, SQLUSMALLINT column) +{ + auto value = cursor.GetString(row, column); + REQUIRE(value.has_value()); + if (!value.has_value()) + throw std::runtime_error("REQUIRE failed but flow continued"); // unreachable + return std::move(*value); +} + } // namespace TEST_CASE_METHOD(SqlTestFixture, "Prefetch: raw GetColumn loop collapses round-trips", "[prefetch]") @@ -639,7 +662,7 @@ TEST_CASE_METHOD(SqlTestFixture, "Prefetch: RowArrayCursor bulk-reads fixed-widt ++blocks; for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock)) { - auto const id = cursor.GetI64(row, 1).value(); + auto const id = RequireI64(cursor, row, 1); // Trim the CHAR(8)/NCHAR(8) driver padding to compare against the trimmed reference. auto code = TrimRight(cursor.GetString(row, 2)); auto wideCode = TrimRight(cursor.GetString(row, 3)); @@ -705,7 +728,7 @@ TEST_CASE_METHOD(SqlTestFixture, for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray()) for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock)) actual.emplace_back( - cursor.GetI64(row, 1).value(), cursor.GetString(row, 2).value(), cursor.GetString(row, 3).value()); + RequireI64(cursor, row, 1), RequireString(cursor, row, 2), RequireString(cursor, row, 3)); } REQUIRE(actual.size() == rowCount); @@ -759,7 +782,7 @@ TEST_CASE_METHOD(SqlTestFixture, { for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock)) { - auto const id = cursor.GetI64(row, 1).value(); + auto const id = RequireI64(cursor, row, 1); ++rowsSeen; if (id % 3 == 0) @@ -815,7 +838,7 @@ TEST_CASE_METHOD(SqlTestFixture, REQUIRE(rowsInFirstBlock == rowCount); // the whole result set fit in one block for (auto const row: std::views::iota(std::size_t { 0 }, rowsInFirstBlock)) { - auto const id = cursor.GetI64(row, 1).value(); + auto const id = RequireI64(cursor, row, 1); CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional { std::format("C{}", id) }); } CHECK(cursor.FetchArray() == 0); // no more rows