-
Notifications
You must be signed in to change notification settings - Fork 203
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
Conversation
It looks like |
… amounts of memory
… SQLite constant is defined
It seems
|
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 |
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 Looking at the implementation of 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:
? |
…urns a hash Works like GC.stat, and can also take a symbol to fetch only one stat
@flavorjones: Done in b9bbeaa. I implemented it like In order to handle the 0 or 1 arguments, I followed the pattern from For tests, I test that I also added documentation for the I believe this covers everything, but let me know if there is something you still want to evolve. |
There was a problem hiding this 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.
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 thestmt_status
interface, except forMEMUSED
, which is accessible viaStatement#memused
.