Skip to content

Conversation

@thisalihassan
Copy link
Contributor

@thisalihassan thisalihassan commented Jan 29, 2026

Summary

  • When binding UTF-8 strings to SQLite statements, transfer ownership of the malloc-backed Utf8Value buffer to SQLite to avoid an extra copy for larger strings.
  • Use sqlite3_bind_blob64() for BLOB parameters to avoid truncation for larger payloads.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Jan 29, 2026
@thisalihassan thisalihassan force-pushed the sqlite-bind-text64-avoid-copy branch from ea75fcc to b7abb6f Compare January 29, 2026 21:00
@thisalihassan
Copy link
Contributor Author

Benchmarks (local, macOS, Release build)

Compared with base branch (main)

Summary:

  • INSERT INTO large_text (text_8kb_column) VALUES (?): *** +5.30% (±1.42%)
  • Other insert cases: within ~±1% (not significant), except all_column_types -0.77% (*) (±0.77%)

change primarily improves large text binds; other workloads appear neutral within measurement noise.

@codecov
Copy link

codecov bot commented Jan 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.77%. Comparing base (37e4004) to head (da58cfd).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61580      +/-   ##
==========================================
- Coverage   89.78%   89.77%   -0.01%     
==========================================
  Files         673      673              
  Lines      203775   203852      +77     
  Branches    39166    39181      +15     
==========================================
+ Hits       182956   183017      +61     
- Misses      13139    13153      +14     
- Partials     7680     7682       +2     
Files with missing lines Coverage Δ
src/node_sqlite.cc 80.75% <100.00%> (+0.09%) ⬆️

... and 45 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thisalihassan thisalihassan force-pushed the sqlite-bind-text64-avoid-copy branch 2 times, most recently from 92bb01c to 8e4016b Compare January 30, 2026 09:23
When binding UTF-8 strings to prepared statements, transfer ownership of
malloc-backed Utf8Value buffers to SQLite to avoid an extra copy for
large strings. Use sqlite3_bind_blob64() when binding BLOB parameters.
@thisalihassan thisalihassan force-pushed the sqlite-bind-text64-avoid-copy branch from 8e4016b to da58cfd Compare January 30, 2026 09:30
@thisalihassan
Copy link
Contributor Author

Hi @cjihrig, @Renegade334 and @mcollina When you have a moment, could you please take a look at this PR and share your feedback? Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants