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

Add some missing wrappers #1139

Merged
merged 9 commits into from
Mar 17, 2022
Merged

Add some missing wrappers #1139

merged 9 commits into from
Mar 17, 2022

Conversation

gwenn
Copy link
Collaborator

@gwenn gwenn commented Mar 15, 2022

  • sqlite3_value_subtype
  • sqlite3_result_subtype
  • sqlite3_db_readonly
  • sqlite3_txn_state
  • sqlite3_stmt_isexplain
  • sqlite3_changes64
  • sqlite3_vtab_config
  • sqlite3_index_info.idxFlags
  • sqlite3_index_info.idxStr
  • sqlite3_index_info.colUsed
  • sqlite3_vtab_collation
  • sqlite3_vtab_distinct
  • sqlite3_vtab_in
  • Fix VTabConfig
  • SQLcipher vs API introduced by 3.38.0: comment them out until SQLcipher is on par ?
  • breaking changes because now modern_sqlite means >= 3.37.0

Fix #598
Also see #639, #1001

sqlite3_bind_*64 vs sqlite3_bind_*
sqlite3_changes64 vs sqlite3_changes
sqlite3_db_readonly
sqlite3_error_offset
sqlite3_index_info.colUsed
sqlite3_index_info.idxFlags
sqlite3_prepare_v3 vs sqlite3_prepare_v2
sqlite3_normalized_sql
sqlite3_stmt_isexplain
sqlite3_result_*64 vs sqlite3_result_*
sqlite3_result_subtype
sqlite3_trace_v2 vs sqlite3_trace
sqlite3_txn_state
sqlite3_value_subtype
sqlite3_value_nochange
sqlite3_value_frombind
sqlite3_vtab_collation
sqlite3_vtab_config
sqlite3_vtab_distinct
sqlite3_vtab_in
sqlite3_vtab_nochange
sqlite3_vtab_rhs_value
sqlite3_value_subtype
sqlite3_result_subtype
sqlite3_changes64
sqlite3_vtab_config
sqlite3_index_info.idxFlags
sqlite3_index_info.colUsed
sqlite3_vtab_distinct
sqlite3_vtab_in
@gwenn gwenn requested a review from thomcc March 15, 2022 19:00
@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #1139 (318ff93) into master (c3b419b) will decrease coverage by 0.21%.
The diff coverage is 63.95%.

@@            Coverage Diff             @@
##           master    #1139      +/-   ##
==========================================
- Coverage   78.43%   78.21%   -0.22%     
==========================================
  Files          47       47              
  Lines        5842     5927      +85     
==========================================
+ Hits         4582     4636      +54     
- Misses       1260     1291      +31     
Impacted Files Coverage Δ
src/config.rs 85.71% <ø> (ø)
src/context.rs 41.17% <ø> (ø)
src/error.rs 15.15% <ø> (ø)
src/functions.rs 78.34% <0.00%> (-1.07%) ⬇️
src/hooks.rs 88.51% <ø> (ø)
src/trace.rs 98.50% <ø> (ø)
src/types/value_ref.rs 50.98% <ø> (ø)
src/vtab/mod.rs 51.19% <38.88%> (-0.56%) ⬇️
src/inner_connection.rs 66.85% <51.72%> (-3.22%) ⬇️
src/vtab/series.rs 83.08% <66.66%> (-0.38%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3b419b...318ff93. Read the comment docs.

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

#[repr(i32)]
#[non_exhaustive]
#[cfg(feature = "modern_sqlite")] // 3.7.7
pub enum VTabConfig {
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to derive at least #[derive(Debug, Clone, Copy, PartialEq)]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will

sqlite3_db_readonly
sqlite3_txn_state
sqlite3_stmt_isexplain
sqlite3_index_info.idxStr
sqlite3_vtab_collation
src/lib.rs Outdated
/// Determine if a database is read-only
#[cfg(feature = "modern_sqlite")] // 3.7.11
#[cfg_attr(docsrs, doc(cfg(feature = "modern_sqlite")))]
pub fn db_readonly(&self, db_name: DatabaseName<'_>) -> Result<bool> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe is_readonly() is a better name for the public API?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right.

/// Determine the transaction state of a database
#[cfg(feature = "modern_sqlite")] // 3.37.0
#[cfg_attr(docsrs, doc(cfg(feature = "modern_sqlite")))]
pub fn txn_state(&self, db_name: Option<crate::DatabaseName<'_>>) -> Result<TransactionState> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe transaction_state is a better name for the public API?

@gwenn gwenn marked this pull request as ready for review March 16, 2022 18:19
@gwenn
Copy link
Collaborator Author

gwenn commented Mar 16, 2022

Could you please tell me how to force github workflow ?

@thomcc
Copy link
Member

thomcc commented Mar 16, 2022

Huh, weird. Not sure why it's not running. You might be able to make it work by going https://github.com/rusqlite/rusqlite/actions/runs/1988699286 and clicking rerun?

@gwenn gwenn closed this Mar 17, 2022
@gwenn gwenn reopened this Mar 17, 2022
@gwenn gwenn merged commit 5e2c103 into rusqlite:master Mar 17, 2022
@gwenn gwenn deleted the missing_wrapper branch March 17, 2022 18: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.

add colused for best_index on virtual tables
2 participants