Skip to content

Commit 198cbd5

Browse files
myrrcmyrrc
authored
New C++ API tests (#8751)
- Add tests for new C++ API. - Upload test results to codecov. - Make vx_array_new_primitive return an error on validity dtype/length mismatch Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
1 parent 159207e commit 198cbd5

29 files changed

Lines changed: 1564 additions & 85 deletions

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ jobs:
215215
# including all private docs.
216216
cargo doc --profile ci --no-deps --document-private-items --workspace --exclude vortex-python --exclude vortex-python-cuda
217217
# nextest doesn't support doc tests, so we run it here
218-
cargo test --profile ci --doc --workspace --all-features --exclude vortex-cxx --exclude vortex-jni --exclude vortex-ffi --exclude xtask --exclude vortex-python-cuda --no-fail-fast
218+
cargo test --profile ci --doc --workspace --all-features --exclude vortex-jni --exclude vortex-ffi --exclude xtask --exclude vortex-python-cuda --no-fail-fast
219219
220220
build-rust:
221221
name: "Rust build (${{matrix.config.name}})"
@@ -561,8 +561,11 @@ jobs:
561561
- name: Build C++ library (asan)
562562
run: |
563563
mkdir -p build
564-
cmake -S lang/cpp -B build -DSANITIZER=asan -DTARGET_TRIPLE="x86_64-unknown-linux-gnu"
564+
cmake -S lang/cpp -Bbuild -DSANITIZER=asan -DBUILD_TESTS=1 -DTARGET_TRIPLE="x86_64-unknown-linux-gnu"
565565
cmake --build build --parallel $(nproc)
566+
- name: Run C++ tests
567+
run: |
568+
build/tests/vortex_cxx_test
566569
567570
sqllogic-test:
568571
name: "SQL logic tests"

‎.github/workflows/rust-instrumented.yml‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ jobs:
101101
--ignore 'home/*' --ignore 'xtask/*' --ignore 'target/*' --ignore 'vortex-error/*' \
102102
--ignore 'vortex-python/*' --ignore 'vortex-jni/*' --ignore 'vortex-flatbuffers/*' \
103103
--ignore 'vortex-proto/*' --ignore 'vortex-tui/*' --ignore 'vortex-datafusion/examples/*' \
104-
--ignore 'vortex-ffi/examples/*' --ignore '*/arbitrary/*' --ignore '*/arbitrary.rs' --ignore 'vortex-cxx/*' \
104+
--ignore 'vortex-ffi/examples/*' --ignore '*/arbitrary/*' --ignore '*/arbitrary.rs' \
105105
--ignore benchmarks/* --ignore 'vortex-test/*' \
106106
-o ${{ env.GRCOV_OUTPUT_FILE }}
107107
test -s ${{ env.GRCOV_OUTPUT_FILE }}
@@ -114,6 +114,42 @@ jobs:
114114
flags: ${{ matrix.suite }}
115115
use_oidc: true
116116

117+
cxx-coverage:
118+
name: "C++ API (coverage)"
119+
timeout-minutes: 15
120+
permissions:
121+
id-token: write
122+
runs-on: >-
123+
${{ github.repository == 'vortex-data/vortex'
124+
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/extras=s3-cache/tag=cxx-coverage', github.run_id)
125+
|| 'ubuntu-latest' }}
126+
steps:
127+
- uses: runs-on/action@v2
128+
if: github.repository == 'vortex-data/vortex'
129+
with:
130+
sccache: s3
131+
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
132+
- uses: ./.github/actions/setup-prebuild
133+
with:
134+
enable-sccache: "true"
135+
- name: Install lcov
136+
run: |
137+
sudo apt-get update
138+
sudo apt-get install -y lcov libjson-xs-perl
139+
- name: Build FFI library
140+
run: cargo build -p vortex-ffi
141+
- name: Build and test C++ API with coverage
142+
working-directory: lang/cpp
143+
run: ./gcov-report.sh
144+
- name: Codecov
145+
uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7
146+
with:
147+
name: cxx-api
148+
files: lang/cpp/coverage.info
149+
disable_search: true
150+
flags: cxx-api
151+
use_oidc: true
152+
117153
rust-test-sanitizer:
118154
strategy:
119155
fail-fast: false

‎docs/api/cpp/index.rst‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,8 @@ Source code for this example is `writer.cpp
164164

165165
.. code-block:: cpp
166166
167-
const Session session;
168-
const DataType dtype = dtype::struct_({
167+
Session session;
168+
DataType dtype = dtype::struct_({
169169
{"age", dtype::uint8()},
170170
{"height", dtype::uint16(Nullable)},
171171
});
@@ -187,15 +187,14 @@ Source code for this example is `writer.cpp
187187
Expression age_gt_10 = expr::gt(expr::col("age"), expr::lit<uint8_t>(10));
188188
Array validity_array = array.apply(age_gt_10);
189189
190-
const Validity validity = Validity::from_array(validity_array);
190+
Validity validity = Validity::from_array(validity_array);
191191
Array array2 = make_struct({
192192
{"age", age},
193193
{"height", Array::primitive<uint16_t>(height_buffer, validity)},
194194
});
195195
196196
Writer writer = Writer::open(session, argv[1], dtype);
197-
writer.push(array);
198-
writer.push(array2);
197+
writer.push({array, array2});
199198
writer.finish();
200199
201200
DataType

‎lang/cpp/.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
coverage*

‎lang/cpp/CMakeLists.txt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ else ()
2121
message(STATUS "Sccache not found")
2222
endif ()
2323

24+
option(BUILD_TESTS "Build tests" OFF)
25+
2426
set(SANITIZER "" CACHE STRING "Build with sanitizers")
2527
set(TARGET_TRIPLE "" CACHE STRING "Rust target triple for FFI library")
2628
set(RUST_BUILD_PROFILE "" CACHE STRING "Cargo profile name for Rust FFI library")
@@ -122,3 +124,24 @@ if(TARGET vortex_ffi_shared)
122124
target_link_libraries(vortex_cxx_shared PRIVATE ${APPLE_LINK_FLAGS})
123125
endif()
124126
endif()
127+
128+
if (BUILD_TESTS)
129+
FetchContent_Declare(
130+
Nanoarrow
131+
GIT_REPOSITORY https://github.com/apache/arrow-nanoarrow
132+
GIT_TAG apache-arrow-nanoarrow-0.8.0
133+
)
134+
FetchContent_MakeAvailable(Nanoarrow)
135+
136+
FetchContent_Declare(
137+
Catch
138+
GIT_REPOSITORY https://github.com/catchorg/Catch2.git
139+
GIT_TAG v3.8.1
140+
)
141+
FetchContent_MakeAvailable(Catch)
142+
include(Catch)
143+
target_compile_definitions(Catch2 PRIVATE CATCH_CONFIG_NO_POSIX_SIGNALS)
144+
145+
include(CTest)
146+
add_subdirectory(tests)
147+
endif()

‎lang/cpp/README.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,11 @@ cmake -Bbuild -DBUILD_EXAMPLES=ON
3535
cmake --build build -j
3636
./build/examples/hello-vortex
3737
```
38+
39+
## Check coverage
40+
41+
This will generate an LCOV directory `coverage`:
42+
43+
```sh
44+
./gcov-report.sh generate
45+
```

‎lang/cpp/gcov-report.sh‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#!/bin/sh
2+
3+
# SPDX-License-Identifier: Apache-2.0
4+
# SPDX-FileCopyrightText: Copyright the Vortex contributors
5+
6+
set -eu
7+
cmake -Bbuild -DBUILD_TESTS=1 -DCMAKE_CXX_FLAGS='-fprofile-arcs -ftest-coverage'
8+
cmake --build build -j
9+
ctest --test-dir build --output-on-failure
10+
11+
geninfo build/CMakeFiles/vortex_cxx_shared.dir/ \
12+
build/tests/CMakeFiles/vortex_cxx_test.dir/ \
13+
--rc geninfo_unexecuted_blocks=1 \
14+
--exclude /usr --exclude build/_deps --exclude tests \
15+
-j -b src -o coverage.info
16+
if [ $# -gt 0 ]; then
17+
genhtml coverage.info -o coverage
18+
fi

‎lang/cpp/include/vortex/array.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ enum class ValidityType {
4040
AllValid = VX_VALIDITY_ALL_VALID,
4141
// All items are invalid
4242
AllInvalid = VX_VALIDITY_ALL_INVALID,
43-
// Item validity is set in a boolean array: true = valid, false = invalid
44-
Array = VX_VALIDITY_ARRAY,
43+
// Item validity is set from a boolean array: true = valid, false = invalid
44+
FromArray = VX_VALIDITY_ARRAY,
4545
};
4646

4747
/**

‎lang/cpp/include/vortex/common.hpp‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33
#pragma once
44

5+
#include <bit>
56
#include <cstdint>
67
#include <span>
78
#include <string_view>
@@ -13,9 +14,34 @@
1314
namespace vortex {
1415
struct float16_t {
1516
uint16_t bits;
16-
friend bool operator==(float16_t, float16_t) = default;
17+
constexpr friend bool operator==(float16_t, float16_t) = default;
1718
// NOLINTNEXTLINE
18-
operator float() const;
19+
constexpr operator float() const {
20+
float result;
21+
const uint32_t sign = (bits >> 15) & 1;
22+
const uint32_t exponent = (bits >> 10) & 0x1F;
23+
const uint32_t mantissa = bits & 0x3FF;
24+
25+
uint32_t out;
26+
if (exponent == 0x1F) {
27+
out = (sign << 31) | 0x7F800000 | (mantissa << 13);
28+
} else if (exponent == 0) {
29+
if (mantissa == 0) {
30+
out = sign << 31;
31+
} else {
32+
uint32_t m = mantissa;
33+
int e = -1;
34+
do {
35+
m <<= 1;
36+
++e;
37+
} while ((m & 0x400) == 0);
38+
out = (sign << 31) | ((127 - 15 - e) << 23) | ((m & 0x3FF) << 13);
39+
}
40+
} else {
41+
out = (sign << 31) | ((exponent - 15 + 127) << 23) | (mantissa << 13);
42+
}
43+
return std::bit_cast<float>(out);
44+
}
1945
};
2046
static_assert(sizeof(float16_t) == 2 && std::is_trivially_copyable_v<float16_t>);
2147
} // namespace vortex
@@ -64,6 +90,10 @@ constexpr vx_ptype to_ptype() {
6490
return PTYPE_I32;
6591
} else if constexpr (std::is_same_v<T, int64_t>) {
6692
return PTYPE_I64;
93+
#if __STDCPP_FLOAT16_T__ == 1
94+
} else if constexpr (std::is_same_v<T, _Float16>) {
95+
return PTYPE_F16;
96+
#endif
6797
} else if constexpr (std::is_same_v<T, float16_t>) {
6898
return PTYPE_F16;
6999
} else if constexpr (std::is_same_v<T, float>) {
@@ -79,7 +109,13 @@ inline constexpr bool is_numeric_element =
79109
std::is_same_v<T, uint8_t> || std::is_same_v<T, uint16_t> || std::is_same_v<T, uint32_t> ||
80110
std::is_same_v<T, uint64_t> || std::is_same_v<T, int8_t> || std::is_same_v<T, int16_t> ||
81111
std::is_same_v<T, int32_t> || std::is_same_v<T, int64_t> || std::is_same_v<T, float> ||
82-
std::is_same_v<T, double> || std::is_same_v<T, float16_t>;
112+
std::is_same_v<T, double>
113+
#if __STDCPP_FLOAT16_T__ == 1
114+
|| std::is_same_v<T, _Float16>
115+
#else
116+
|| std::is_same_v<T, float16_t>
117+
#endif
118+
;
83119
} // namespace vortex::detail
84120

85121
namespace vortex {

‎lang/cpp/include/vortex/scan.hpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@
2020
namespace vortex {
2121

2222
namespace detail {
23+
2324
// range-for support for Scan and Partition
24-
template <class Source, class Item, auto Next>
25+
template <class Source, class Item, std::optional<Item> (Source::*Next)()>
2526
class PullRange {
2627
public:
2728
class iterator {
@@ -37,7 +38,7 @@ class PullRange {
3738
return *cur_;
3839
}
3940
iterator &operator++() {
40-
cur_ = Next(src_);
41+
cur_ = (src_->*Next)();
4142
return *this;
4243
}
4344
void operator++(int) {
@@ -55,7 +56,7 @@ class PullRange {
5556
explicit PullRange(Source &src) : src_(&src) {
5657
}
5758
iterator begin() {
58-
return iterator(src_, Next(src_));
59+
return iterator(src_, (src_->*Next)());
5960
}
6061
std::default_sentinel_t end() {
6162
return std::default_sentinel;

0 commit comments

Comments
 (0)