Skip to content

Scalar function coverage #1

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

Merged
merged 18 commits into from
Nov 27, 2024

Conversation

taniabogatsch
Copy link
Collaborator

This PR covers all ...[scalar]... functions in the C API.
It also includes:

  • some minor folder reorganization.
  • slightly less verbose function names for the tests.
  • duckdb_state returns state when registering the various scalar functions (still a big ugly...).

The following would help with building this reference extension.

  • Python script to get the diff between the covered functions and the extension function struct. These are the functions that still need to be covered.
  • A formatter.

I'll also throw the command to get some debugging going in here.

lldb ./configure/venv/bin/python3 -- -m duckdb_sqllogictest --test-dir test/sql  --external-extension build/debug/reference_extension_c.duckdb_extension

Copy link
Collaborator

@samansmink samansmink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

@samansmink
Copy link
Collaborator

I can look into adding this to the template/ci tools:

  • formatter
  • makefile recipe to run tests in debugger

The script for detecting coverage is definitely something we need, not sure how to tackle this. I see a few options:

  • python parsing script that just checks presence of function name (a little hacky perhaps?
  • instrumentation DuckDB side that logs C API function calls
  • instrumentation Extension side that logs C API function calls

maybe its not such a bad idea to have debug builds of DuckDB have the option of logging every C API call that is made. It will probably be really slow but it might be nice to have anyway? Ofc this would depend on the logging infrastructure to be there in time

@taniabogatsch
Copy link
Collaborator Author

Great, yeah, the debugger and the formatter will be handy.

Should we also slow down our additions here until we know which functions will be stable? The scalar function API and basic data chunk handling should be fine; we'll see.

W.r.t. the logging, this is the cleanest solution. However, the Python parsing script should suffice until this repo/extension is close to completion. :)

@samansmink samansmink merged commit 75e4e67 into duckdb:main Nov 27, 2024
11 checks passed
@taniabogatsch taniabogatsch deleted the scalar-function-coverage branch November 27, 2024 14:28
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.

2 participants