Skip to content

Commit 836ffa7

Browse files
YaraslautYaraslaut
authored
Add RowArrayCursor fixed-width prefetch tests and Codecov CI job (#518)
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.
2 parents 4601129 + a0ba94f commit 836ffa7

3 files changed

Lines changed: 365 additions & 1 deletion

File tree

‎.github/workflows/build.yml‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,3 +927,52 @@ jobs:
927927
./out/build/clang-${{ matrix.sanitizer }}/src/tests/LightweightTest --test-env=sqlite3
928928
929929
# }}}
930+
# {{{ Coverage
931+
coverage:
932+
name: "Code coverage"
933+
runs-on: ubuntu-24.04
934+
steps:
935+
- uses: actions/checkout@v6
936+
- name: Fetch tags
937+
run: git fetch --prune --unshallow --tags
938+
- name: ccache
939+
uses: hendrikmuhs/ccache-action@v1.2
940+
with:
941+
key: "ccache-ubuntu2404-clang-coverage"
942+
max-size: 256M
943+
- name: "update APT database"
944+
run: sudo apt -q update
945+
- name: Install Clang
946+
run: |
947+
wget https://apt.llvm.org/llvm.sh
948+
chmod +x llvm.sh
949+
sudo ./llvm.sh ${{ env.CLANG_TOOLS_VERSION }}
950+
- name: "install dependencies"
951+
# Rename the libsqliteodbc driver section to match the Windows driver
952+
# name so the shared .test-env.yml works on both platforms.
953+
run: |
954+
sudo apt install -y cmake ninja-build catch2 unixodbc-dev sqlite3 libsqlite3-dev libsqliteodbc uuid-dev libyaml-cpp-dev libzip-dev lcov
955+
sudo sed -i 's/^\[SQLite3\]$/[SQLite3 ODBC Driver]/' /etc/odbcinst.ini
956+
- name: "cmake"
957+
run: |
958+
cmake --preset clang-coverage \
959+
-D CMAKE_CXX_COMPILER=clang++-${{ env.CLANG_TOOLS_VERSION }} \
960+
-D CMAKE_C_COMPILER=clang-${{ env.CLANG_TOOLS_VERSION }}
961+
- name: "build"
962+
run: cmake --build --preset clang-coverage -- -j$(nproc)
963+
# The `coverage` target (cmake/Coverage.cmake) runs the suite against sqlite3/postgres/mssql
964+
# (the latter two tolerate connection failure via `|| true`) and captures lcov data. Only
965+
# sqlite3 is reachable in this job (no Docker services here), matching the "Sanitizers" job's
966+
# scope — the full 3-database matrix is exercised, uninstrumented, by dbms_test_matrix; this
967+
# job's purpose is a coverage percentage, not additional correctness coverage.
968+
- name: "Run tests and generate lcov report"
969+
run: cmake --build --preset clang-coverage --target coverage
970+
- name: "Upload coverage to Codecov"
971+
uses: codecov/codecov-action@v5
972+
with:
973+
token: ${{ secrets.CODECOV_TOKEN }}
974+
files: ./out/build/clang-coverage/coverage/coverage.info
975+
fail_ci_if_error: false
976+
verbose: true
977+
978+
# }}}

‎cmake/Coverage.cmake‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,22 @@ endif()
5656
# For Clang, we need to use llvm-cov gcov instead of gcov
5757
set(GCOV_TOOL_OPTION "")
5858
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
59-
find_program(LLVM_COV_PATH llvm-cov)
59+
# apt.llvm.org's installer (llvm.sh) only ever installs version-suffixed binaries
60+
# (llvm-cov-22, never a bare llvm-cov symlink), so an unversioned lookup silently
61+
# fails and lcov falls back to the system gcov (from GCC), which cannot parse
62+
# Clang's gcov-compatible data format ("Incompatible GCC/GCOV version" from
63+
# geninfo). Try the version matching the active compiler first, then fall back
64+
# to a few recent majors and finally the unversioned name for other toolchains
65+
# (e.g. Homebrew LLVM on macOS, which does install a bare llvm-cov).
66+
if(CMAKE_CXX_COMPILER_VERSION)
67+
string(REGEX MATCH "^[0-9]+" LLVM_COV_VERSION_SUFFIX "${CMAKE_CXX_COMPILER_VERSION}")
68+
endif()
69+
find_program(LLVM_COV_PATH
70+
NAMES
71+
"llvm-cov-${LLVM_COV_VERSION_SUFFIX}"
72+
llvm-cov-22 llvm-cov-21 llvm-cov-20 llvm-cov-19 llvm-cov-18
73+
llvm-cov
74+
)
6075
if(LLVM_COV_PATH)
6176
# Create a wrapper script for llvm-cov gcov
6277
set(GCOV_WRAPPER_SCRIPT "${CMAKE_BINARY_DIR}/llvm-gcov-wrapper.sh")

‎src/tests/SqlStatementPrefetchTests.cpp‎

Lines changed: 300 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,40 @@ SqlStatement MakePerRowStatement()
122122
return stmt;
123123
}
124124

125+
// Strips the server's CHAR(N)/NCHAR(N) right-padding (trailing ASCII spaces) so a fixed-width
126+
// column's value compares equal regardless of how much padding the driver returned it with.
127+
std::optional<std::string> TrimRight(std::optional<std::string> value)
128+
{
129+
if (!value.has_value())
130+
return std::nullopt;
131+
while (!value->empty() && value->back() == ' ')
132+
value->pop_back();
133+
return value;
134+
}
135+
136+
// Unwraps a RowArrayCursor cell known to be non-NULL (e.g. the primary key column, or a text
137+
// column the test never inserts NULL into), REQUIRE-ing that first. The explicit if-with-throw is
138+
// what clang-tidy's bugprone-unchecked-optional-access analysis recognizes as a null check —
139+
// REQUIRE alone is a macro it cannot reason about (see the same pattern in SqlGuidTests.cpp's
140+
// RequireParsed).
141+
std::int64_t RequireI64(RowArrayCursor const& cursor, std::size_t row, SQLUSMALLINT column)
142+
{
143+
auto const value = cursor.GetI64(row, column);
144+
REQUIRE(value.has_value());
145+
if (!value.has_value())
146+
throw std::runtime_error("REQUIRE failed but flow continued"); // unreachable
147+
return *value;
148+
}
149+
150+
std::string RequireString(RowArrayCursor const& cursor, std::size_t row, SQLUSMALLINT column)
151+
{
152+
auto value = cursor.GetString(row, column);
153+
REQUIRE(value.has_value());
154+
if (!value.has_value())
155+
throw std::runtime_error("REQUIRE failed but flow continued"); // unreachable
156+
return std::move(*value);
157+
}
158+
125159
} // namespace
126160

127161
TEST_CASE_METHOD(SqlTestFixture, "Prefetch: raw GetColumn loop collapses round-trips", "[prefetch]")
@@ -544,6 +578,272 @@ TEST_CASE_METHOD(SqlTestFixture, "Prefetch: GUID column round-trips", "[prefetch
544578
REQUIRE(actual.size() == rowCount);
545579
}
546580

581+
TEST_CASE_METHOD(SqlTestFixture, "Prefetch: RowArrayCursor bulk-reads fixed-width CHAR/NCHAR columns", "[prefetch]")
582+
{
583+
// Fixed-width text (CHAR(N)/NCHAR(N)) is excluded from the transparent automatic prefetch gate
584+
// (PrefetchableSqlType in SqlStatement.cpp) and always falls back to per-row SQLGetData there.
585+
// RowArrayCursor — the actual bulk block-fetch mechanism reachable via ExecuteBatchFetch — binds
586+
// and reads fixed-width text directly, independent of that gate. This pins that a bulk fetch of
587+
// CHAR(N)/NCHAR(N) columns, including NULLs and the server's right-padding, is byte-identical to
588+
// the trusted per-row reference.
589+
auto stmt = SqlStatement {};
590+
591+
stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) {
592+
migration.CreateTable("PrefetchFixedWidth")
593+
.PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {})
594+
.Column("Code", SqlColumnTypeDefinitions::Char { 8 }) // narrow fixed-width, right-padded
595+
.Column("WideCode", SqlColumnTypeDefinitions::NChar { 8 }); // wide fixed-width, right-padded
596+
});
597+
598+
constexpr std::size_t rowCount = TestPrefetchDepth + 6;
599+
stmt.Prepare(stmt.Query("PrefetchFixedWidth")
600+
.Insert()
601+
.Set("Id", SqlWildcard)
602+
.Set("Code", SqlWildcard)
603+
.Set("WideCode", SqlWildcard));
604+
for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1))
605+
{
606+
// Every 4th row is NULL in both fixed-width columns; the rest carry a short value shorter
607+
// than the column's declared width, so the server's right-padding is actually exercised.
608+
if (i % 4 == 0)
609+
(void) stmt.Execute(static_cast<std::int64_t>(i), std::optional<std::string> {}, std::optional<std::string> {});
610+
else
611+
(void) stmt.Execute(static_cast<std::int64_t>(i),
612+
std::optional<std::string> { std::format("C{}", i % 100) },
613+
std::optional<std::string> { std::format("W{}", i % 100) });
614+
}
615+
616+
// Trusted reference: the classic per-row SQLGetData path. SqlTrimmedFixedString strips the
617+
// server's CHAR(N) right-padding on the narrow column; the wide column is read as plain
618+
// std::string (already-proven UTF-16 -> UTF-8 conversion) and trimmed manually here to mirror
619+
// RowArrayCursor::GetString's own trailing-space trim below, since SqlTrimmedWideFixedString's
620+
// GetColumn does not currently apply its trim (a separate, pre-existing gap outside this test's
621+
// scope — see SqlDataBinder<Utf16StringType>::GetColumn in BasicStringBinder.hpp).
622+
using Row = std::tuple<std::int64_t, std::optional<std::string>, std::optional<std::string>>;
623+
auto readPerRow = [&]() {
624+
std::vector<Row> rows;
625+
auto reference = MakePerRowStatement();
626+
auto cursor =
627+
reference.ExecuteDirect(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedWidth" ORDER BY "Id")"sv);
628+
while (cursor.FetchRow())
629+
{
630+
auto const id = cursor.GetColumn<std::int64_t>(1);
631+
auto const code = cursor.GetNullableColumn<SqlTrimmedFixedString<8>>(2);
632+
auto const wideCode = TrimRight(cursor.GetNullableColumn<std::string>(3));
633+
rows.emplace_back(
634+
id, code.has_value() ? std::optional<std::string> { code->ToStringView() } : std::nullopt, wideCode);
635+
}
636+
return rows;
637+
};
638+
auto const expected = readPerRow();
639+
REQUIRE(expected.size() == rowCount);
640+
641+
// RowArrayCursor directly — the bulk block-fetch mechanism under test.
642+
std::vector<Row> actual;
643+
{
644+
auto cursor = stmt.ExecuteBatchFetch(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedWidth" ORDER BY "Id")"sv,
645+
TestPrefetchDepth);
646+
647+
// Confirm the columns actually bound as text (Char/WChar), not silently as something else.
648+
// Which of Char/WChar a driver picks for a given declared SQL type is driver-specific and does
649+
// not affect correctness (RowArrayCursor::GetString normalizes both to UTF-8) — e.g. SQLite3's
650+
// ODBC driver reports CHAR(N) as WChar on Windows but as Char on Linux, and NCHAR(N) the other
651+
// way around on Linux vs Windows. Accept either for both columns; only the byte-identity check
652+
// below actually matters.
653+
auto const isTextlike = [](RowArrayCursor::BoundType type) {
654+
return type == RowArrayCursor::BoundType::Char || type == RowArrayCursor::BoundType::WChar;
655+
};
656+
CHECK(isTextlike(cursor.ColumnBoundType(2)));
657+
CHECK(isTextlike(cursor.ColumnBoundType(3)));
658+
659+
std::size_t blocks = 0;
660+
for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray())
661+
{
662+
++blocks;
663+
for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock))
664+
{
665+
auto const id = RequireI64(cursor, row, 1);
666+
// Trim the CHAR(8)/NCHAR(8) driver padding to compare against the trimmed reference.
667+
auto code = TrimRight(cursor.GetString(row, 2));
668+
auto wideCode = TrimRight(cursor.GetString(row, 3));
669+
actual.emplace_back(id, std::move(code), std::move(wideCode));
670+
}
671+
}
672+
CHECK(blocks >= 2); // crossed at least one block boundary — genuinely block-fetched, not one row
673+
}
674+
675+
REQUIRE(actual.size() == rowCount);
676+
CHECK(actual == expected); // byte-identical to the trusted per-row path, including NULLs and padding
677+
}
678+
679+
TEST_CASE_METHOD(SqlTestFixture,
680+
"Prefetch: RowArrayCursor bulk-reads a fixed-width value that fills its full capacity",
681+
"[prefetch]")
682+
{
683+
// Regression sibling to #485 (BasicStringBinder::OutputColumn off-by-one on BufferLength) for the
684+
// RowArrayCursor bulk path: a value with NO trailing padding — it fills CHAR(N)/NCHAR(N) exactly —
685+
// must not lose its last character to a buffer sized for the terminator alone.
686+
auto stmt = SqlStatement {};
687+
688+
stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) {
689+
migration.CreateTable("PrefetchFixedFull")
690+
.PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {})
691+
.Column("Code", SqlColumnTypeDefinitions::Char { 8 })
692+
.Column("WideCode", SqlColumnTypeDefinitions::NChar { 8 });
693+
});
694+
695+
constexpr std::size_t rowCount = TestPrefetchDepth + 6;
696+
stmt.Prepare(stmt.Query("PrefetchFixedFull")
697+
.Insert()
698+
.Set("Id", SqlWildcard)
699+
.Set("Code", SqlWildcard)
700+
.Set("WideCode", SqlWildcard));
701+
for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1))
702+
{
703+
// Every value is exactly 8 chars — fills CHAR(8)/NCHAR(8) with no padding at all.
704+
auto const code = std::format("{:08}", i % 100'000'000);
705+
(void) stmt.Execute(static_cast<std::int64_t>(i), code, code);
706+
}
707+
708+
auto readPerRow = [&]() {
709+
std::vector<std::tuple<std::int64_t, std::string, std::string>> rows;
710+
auto reference = MakePerRowStatement();
711+
auto cursor = reference.ExecuteDirect(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedFull" ORDER BY "Id")"sv);
712+
while (cursor.FetchRow())
713+
{
714+
auto const id = cursor.GetColumn<std::int64_t>(1);
715+
auto const code = cursor.GetColumn<SqlTrimmedFixedString<8>>(2);
716+
auto const wideCode = *TrimRight(cursor.GetColumn<std::string>(3));
717+
rows.emplace_back(id, std::string { code.ToStringView() }, wideCode);
718+
}
719+
return rows;
720+
};
721+
auto const expected = readPerRow();
722+
REQUIRE(expected.size() == rowCount);
723+
724+
std::vector<std::tuple<std::int64_t, std::string, std::string>> actual;
725+
{
726+
auto cursor = stmt.ExecuteBatchFetch(R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedFull" ORDER BY "Id")"sv,
727+
TestPrefetchDepth);
728+
for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray())
729+
for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock))
730+
actual.emplace_back(
731+
RequireI64(cursor, row, 1), RequireString(cursor, row, 2), RequireString(cursor, row, 3));
732+
}
733+
734+
REQUIRE(actual.size() == rowCount);
735+
for (std::size_t i = 0; i < rowCount; ++i)
736+
{
737+
CHECK(std::get<0>(actual[i]) == std::get<0>(expected[i]));
738+
CHECK(std::get<1>(actual[i]) == std::get<1>(expected[i])); // full 8 chars, not truncated to 7
739+
CHECK(std::get<2>(actual[i]) == std::get<2>(expected[i]));
740+
CHECK(std::get<1>(actual[i]).size() == 8);
741+
CHECK(std::get<2>(actual[i]).size() == 8);
742+
}
743+
}
744+
745+
TEST_CASE_METHOD(SqlTestFixture,
746+
"Prefetch: RowArrayCursor distinguishes NULL from empty string in fixed-width columns",
747+
"[prefetch]")
748+
{
749+
// NULL and "" are distinct SQL values; a bulk fetch must not collapse one into the other (e.g. by
750+
// treating an all-blank buffer as NULL, or a NULL indicator as an empty-but-non-null string).
751+
auto stmt = SqlStatement {};
752+
753+
stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) {
754+
migration.CreateTable("PrefetchFixedEmptyVsNull")
755+
.PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {})
756+
.Column("Code", SqlColumnTypeDefinitions::Char { 8 })
757+
.Column("WideCode", SqlColumnTypeDefinitions::NChar { 8 });
758+
});
759+
760+
constexpr std::size_t rowCount = TestPrefetchDepth + 6;
761+
stmt.Prepare(stmt.Query("PrefetchFixedEmptyVsNull")
762+
.Insert()
763+
.Set("Id", SqlWildcard)
764+
.Set("Code", SqlWildcard)
765+
.Set("WideCode", SqlWildcard));
766+
for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1))
767+
{
768+
// Every 3rd row is NULL, every other 3rd row is an empty string, the rest carry a short value.
769+
if (i % 3 == 0)
770+
(void) stmt.Execute(static_cast<std::int64_t>(i), std::optional<std::string> {}, std::optional<std::string> {});
771+
else if (i % 3 == 1)
772+
(void) stmt.Execute(static_cast<std::int64_t>(i), std::string {}, std::string {});
773+
else
774+
(void) stmt.Execute(static_cast<std::int64_t>(i), std::format("C{}", i), std::format("W{}", i));
775+
}
776+
777+
auto cursor = stmt.ExecuteBatchFetch(
778+
R"(SELECT "Id", "Code", "WideCode" FROM "PrefetchFixedEmptyVsNull" ORDER BY "Id")"sv, TestPrefetchDepth);
779+
780+
std::size_t rowsSeen = 0;
781+
for (auto rowsInBlock = cursor.FetchArray(); rowsInBlock > 0; rowsInBlock = cursor.FetchArray())
782+
{
783+
for (auto const row: std::views::iota(std::size_t { 0 }, rowsInBlock))
784+
{
785+
auto const id = RequireI64(cursor, row, 1);
786+
++rowsSeen;
787+
788+
if (id % 3 == 0)
789+
{
790+
// NULL: the indicator must say so, and GetString must not fabricate a value.
791+
CHECK(cursor.IsCellNull(row, 2));
792+
CHECK(cursor.IsCellNull(row, 3));
793+
CHECK_FALSE(cursor.GetString(row, 2).has_value());
794+
CHECK_FALSE(cursor.GetString(row, 3).has_value());
795+
}
796+
else if (id % 3 == 1)
797+
{
798+
// Empty string, server-padded to CHAR(8)/NCHAR(8): NOT null, and trims down to "".
799+
CHECK_FALSE(cursor.IsCellNull(row, 2));
800+
CHECK_FALSE(cursor.IsCellNull(row, 3));
801+
CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional<std::string> { "" });
802+
CHECK(TrimRight(cursor.GetString(row, 3)) == std::optional<std::string> { "" });
803+
}
804+
else
805+
{
806+
CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional<std::string> { std::format("C{}", id) });
807+
CHECK(TrimRight(cursor.GetString(row, 3)) == std::optional<std::string> { std::format("W{}", id) });
808+
}
809+
}
810+
}
811+
CHECK(rowsSeen == rowCount);
812+
}
813+
814+
TEST_CASE_METHOD(SqlTestFixture,
815+
"Prefetch: RowArrayCursor bulk-reads fixed-width columns within a single block",
816+
"[prefetch]")
817+
{
818+
// The other fixed-width tests deliberately cross a block boundary; this pins the same correctness
819+
// for a result set that fits in a single FetchArray() call (arrayDepth > rowCount), so the
820+
// single-block code paths (no continuation, no second SQLGetData) are exercised too.
821+
auto stmt = SqlStatement {};
822+
823+
stmt.MigrateDirect([](SqlMigrationQueryBuilder& migration) {
824+
migration.CreateTable("PrefetchFixedSingleBlock")
825+
.PrimaryKey("Id", SqlColumnTypeDefinitions::Bigint {})
826+
.Column("Code", SqlColumnTypeDefinitions::Char { 8 });
827+
});
828+
829+
constexpr std::size_t rowCount = 5; // well under TestPrefetchDepth
830+
stmt.Prepare(stmt.Query("PrefetchFixedSingleBlock").Insert().Set("Id", SqlWildcard).Set("Code", SqlWildcard));
831+
for (auto const i: std::views::iota(std::size_t { 1 }, rowCount + 1))
832+
(void) stmt.Execute(static_cast<std::int64_t>(i), std::format("C{}", i));
833+
834+
auto cursor =
835+
stmt.ExecuteBatchFetch(R"(SELECT "Id", "Code" FROM "PrefetchFixedSingleBlock" ORDER BY "Id")"sv, TestPrefetchDepth);
836+
837+
auto const rowsInFirstBlock = cursor.FetchArray();
838+
REQUIRE(rowsInFirstBlock == rowCount); // the whole result set fit in one block
839+
for (auto const row: std::views::iota(std::size_t { 0 }, rowsInFirstBlock))
840+
{
841+
auto const id = RequireI64(cursor, row, 1);
842+
CHECK(TrimRight(cursor.GetString(row, 2)) == std::optional<std::string> { std::format("C{}", id) });
843+
}
844+
CHECK(cursor.FetchArray() == 0); // no more rows
845+
}
846+
547847
// Opt-in micro-benchmark: per-row SQLFetch vs transparent block-prefetch over a large result set.
548848
// Run with: LightweightTest "[prefetchbench]" (hidden by the leading '.').
549849
TEST_CASE_METHOD(SqlTestFixture, "Prefetch benchmark", "[.][prefetchbench]")

0 commit comments

Comments
 (0)