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

Implement sqlite3_stmt_status interface #461

Merged
merged 13 commits into from Jan 19, 2024

Conversation

fractaledmind
Copy link
Contributor

@fractaledmind fractaledmind commented Jan 8, 2024

Resolves #410

This PR adds 2 instance methods to the Statement class:

  • Statement#stat
  • Statement#memused

Statement#stat returns a hash with each of the counters available to the stmt_status interface, except for MEMUSED, which is accessible via Statement#memused.

@fractaledmind
Copy link
Contributor Author

It looks like system installations don't have every statement counter constant. Some guidance on how best to account for undefined constants would be appreciated. Should I simply check and not define the method? Should I define the method, but have the body raise something like a NoMethodError? Should I do something else entirely?

@fractaledmind
Copy link
Contributor Author

It seems ubuntu-latest, 3.3, system runs SQLite v3.37.2. Bloom filters and thus the SQLITE_STMTSTATUS_FILTER_HIT and SQLITE_STMTSTATUS_FILTER_MISS counters were added in v3.38.0.

sqlcipher runs SQLite v3.15.2. That version is missing SQLITE_STMTSTATUS_REPREPARE, SQLITE_STMTSTATUS_RUN, SQLITE_STMTSTATUS_FILTER_MISS, SQLITE_STMTSTATUS_FILTER_HIT, and SQLITE_STMTSTATUS_MEMUSED. I have made each of those corresponding functions optionally defined.

@fractaledmind
Copy link
Contributor Author

I don't know what it means when GitHub Actions don't have any output, they just show a box that says "This job failed", but I know that there were a number of actions running concurrently last night from other PRs, so maybe it timed out.

@flavorjones @tenderlove: If you re-run CI, I expect everything to pass

@flavorjones
Copy link
Member

Hey! Thanks for opening this up. The code as written seems great.

I have a question about the API design, though: as sqlite continues to add new stats, we'll be adding new methods each time, which feels strange to me. Some existing APIs that have a similar purpose provide a single method that returns a hash of statistics, for example GC.stat.

Looking at the implementation of sqlite3_stmt_status we can see that all the stats except MEMUSED only require some pointer dereferences, and so are going to be fast and non-blocking; and MEMUSED acquires a mutex on the database:

int sqlite3_stmt_status(sqlite3_stmt *pStmt, int op, int resetFlag){
  Vdbe *pVdbe = (Vdbe*)pStmt;
  u32 v;
  // ... snip ... 
  if( op==SQLITE_STMTSTATUS_MEMUSED ){
    sqlite3 *db = pVdbe->db;
    sqlite3_mutex_enter(db->mutex);
    v = 0;
    db->pnBytesFreed = (int*)&v;
    assert( db->lookaside.pEnd==db->lookaside.pTrueEnd );
    db->lookaside.pEnd = db->lookaside.pStart;
    sqlite3VdbeDelete(pVdbe);
    db->pnBytesFreed = 0;
    db->lookaside.pEnd = db->lookaside.pTrueEnd;
    sqlite3_mutex_leave(db->mutex);
  }else{
    v = pVdbe->aCounter[op];
    if( resetFlag ) pVdbe->aCounter[op] = 0;
  }
  return (int)v;
}

What do you think of making this two methods:

  • Statement#stat that returns a hash containing all the supported statistics except MEMUSED
  • Statement#memused that returns an int

?

…urns a hash

Works like GC.stat, and can also take a symbol to fetch only one stat
@fractaledmind
Copy link
Contributor Author

@flavorjones: Done in b9bbeaa.

I implemented it like GC.stat, so you can pass no argument and just get a hash of stats back, or you can pass a hash, which will have the stats added to it, or you can pass a symbol for a particular stat and only that stat will be computed and returned to you.

In order to handle the 0 or 1 arguments, I followed the pattern from db_filename; that is, I defined a C method called stmt_stat which requires 1 argument that can be nil, Hash, or Symbol. I then define a Statement#stat method in Ruby which use a default argument to allow no argument to be passed.

For tests, I test that stmt.stat returns a Hash and then I just adapted the original tests to test that stmt.stat(key) returns the correct calculation. To handle some stats not being present in some builds, each stat test calls stmt.stat to get the hash of stats to see if the key is present.

I also added documentation for the Statement#stat method defined in Ruby.

I believe this covers everything, but let me know if there is something you still want to evolve.

Copy link
Member

@tenderlove tenderlove left a comment

Choose a reason for hiding this comment

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

I think we should make the C function stmt_stat private. I left one suggestion, but it's not required.

ext/sqlite3/statement.c Outdated Show resolved Hide resolved
lib/sqlite3/statement.rb Outdated Show resolved Hide resolved
@tenderlove tenderlove merged commit 6593767 into sparklemotion:main Jan 19, 2024
99 checks passed
@fractaledmind fractaledmind deleted the stmt-status branch January 21, 2024 17:44
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.

Implement the sqlite3_stmt_status interface?
3 participants