Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link C and C++ API tests against the API-restricted shared library #2153

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

benjaminwinger
Copy link
Collaborator

@benjaminwinger benjaminwinger commented Oct 5, 2023

First pass at this, mostly to see if it passes CI.

Implements part of #2136.

Various functions needed the KUZU_API annotation. Some of these are fine, such as ~PreparedStatement and StorageDriver, which should be public.

However a number of things which should really be private (e.g. ValueVector::resetAuxiliaryBuffer) had to be added to the exports since they are used in template functions related to the UDF helpers. While the templates themselves can't be moved into source files, we could probably hide some of their implementations, but I'm not sure if that really does anything other than expose something different which really should be private.

I'm also not convinced that ValueVector was really written with external use in mind, as it has various public functions which users probably shouldn't be calling. However it is required for Connection::createVectorizedFunction to work. I've only added KUZU_API to the bare minimum functions, but note that all inline functions (including getData() for direct access to the buffer) would be usable.

TODO:

  • InMemOverflowBuffer::allocateSpace could be removed from the exports if the various operation functions are moved into cpp files. Done; I threw them into vector_string_functions.cpp, though I'm not sure if that's ideal. They could have their own files, but the structure of src/function doesn't match src/include/function and I wasn't sure where they should go.

@codecov
Copy link

codecov bot commented Oct 5, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (37800fe) 89.63% compared to head (e0507dd) 89.67%.
Report is 40 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2153      +/-   ##
==========================================
+ Coverage   89.63%   89.67%   +0.04%     
==========================================
  Files         997      989       -8     
  Lines       35729    35389     -340     
==========================================
- Hits        32025    31735     -290     
+ Misses       3704     3654      -50     
Files Coverage Δ
src/common/types/types.cpp 92.13% <100.00%> (+0.08%) ⬆️
src/common/vector/value_vector.cpp 95.16% <100.00%> (+0.07%) ⬆️
src/include/common/data_chunk/sel_vector.h 100.00% <ø> (ø)
src/include/common/types/types.h 100.00% <ø> (ø)
src/include/common/vector/value_vector.h 100.00% <100.00%> (ø)
...nclude/function/string/functions/concat_function.h 100.00% <ø> (ø)
...nclude/function/string/functions/repeat_function.h 100.00% <ø> (ø)
...clude/function/string/functions/reverse_function.h 100.00% <ø> (+4.34%) ⬆️
src/include/main/connection.h 100.00% <100.00%> (ø)
src/include/main/prepared_statement.h 100.00% <ø> (ø)
... and 1 more

... and 149 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ray6080 ray6080 self-requested a review October 11, 2023 02:42
src/include/main/connection.h Show resolved Hide resolved
@benjaminwinger benjaminwinger merged commit 10233ad into master Oct 12, 2023
11 checks passed
@benjaminwinger benjaminwinger deleted the api-tests branch October 12, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants