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 statement_timeout= method #463

Merged
merged 22 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6c4e155
Add the progress_handler interface
fractaledmind Jan 7, 2024
046bd67
Add tests for the progress_handler interface
fractaledmind Jan 7, 2024
11a94dc
Merge branch 'main' into progress-handler
fractaledmind Jan 7, 2024
50f1ef0
Remove extra line
fractaledmind Jan 7, 2024
c6bf834
Bump the frequency for the opcode arg test
fractaledmind Jan 7, 2024
3681b2a
Bump the frequency for the opcode arg test and make assertion more re…
fractaledmind Jan 7, 2024
c0d5144
Only define the ruby progress_handler method if the OMIT compilation …
fractaledmind Jan 7, 2024
e3f95ca
Add statement_timeout= method that leverages progress_handler to inte…
fractaledmind Jan 8, 2024
3ebee19
Merge branch 'progress-handler' into statement-timeout
fractaledmind Jan 8, 2024
b819265
Add a test for statement_timeout=
fractaledmind Jan 8, 2024
3ba847c
Allow statement_timeout= to accept a nil to reset the progress_handler
fractaledmind Jan 8, 2024
870f3b9
Merge branch 'master' into statement-timeout
fractaledmind Jan 11, 2024
bf1f0e6
Implement that statement_timeout= in pure C
fractaledmind Jan 11, 2024
c8007e7
Remove old progress_handler tests
fractaledmind Jan 11, 2024
ae76a01
Update ext/sqlite3/database.c
fractaledmind Jan 11, 2024
cd47d9b
Reset stmt_deadline when preparing a statement
fractaledmind Jan 11, 2024
13d4067
Make test faster
fractaledmind Jan 11, 2024
31e44c1
Merge branch 'main' into statement-timeout
fractaledmind Jan 21, 2024
bd0267e
Implement statement_timeout with timespecs
fractaledmind Jan 21, 2024
6456dec
Add the new timespec.h file to the gemspec
fractaledmind Jan 22, 2024
832fe6a
Update test-gem-file-contents
fractaledmind Jan 22, 2024
c80aeef
Update ext/sqlite3/database.c
fractaledmind Jan 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bin/test-gem-file-contents
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe File.basename(gemfile) do

it "contains extension C and header files" do
assert_equal(6, gemfile_contents.count { |f| File.fnmatch?("ext/**/*.c", f) })
assert_equal(6, gemfile_contents.count { |f| File.fnmatch?("ext/**/*.h", f) })
assert_equal(7, gemfile_contents.count { |f| File.fnmatch?("ext/**/*.h", f) })
end

it "includes C files in extra_rdoc_files" do
Expand Down Expand Up @@ -154,7 +154,7 @@ describe File.basename(gemfile) do

it "contains extension C and header files" do
assert_equal(6, gemfile_contents.count { |f| File.fnmatch?("ext/**/*.c", f) })
assert_equal(6, gemfile_contents.count { |f| File.fnmatch?("ext/**/*.h", f) })
assert_equal(7, gemfile_contents.count { |f| File.fnmatch?("ext/**/*.h", f) })
end

it "includes C files in extra_rdoc_files" do
Expand Down
41 changes: 41 additions & 0 deletions ext/sqlite3/database.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,44 @@ busy_handler(int argc, VALUE *argv, VALUE self)
return self;
}

static int
rb_sqlite3_statement_timeout(void *context)
{
sqlite3RubyPtr ctx = (sqlite3RubyPtr)context;
struct timespec currentTime;
clock_gettime(CLOCK_MONOTONIC, &currentTime);

if (!timespecisset(&ctx->stmt_deadline)) {
// Set stmt_deadline if not already set
ctx->stmt_deadline = currentTime;
} else if (timespecafter(&currentTime, &ctx->stmt_deadline)) {
return 1;
}

return 0;
}

/* call-seq: db.statement_timeout = ms
*
* Indicates that if a query lasts longer than the indicated number of
* milliseconds, SQLite should interrupt that query and return an error.
* By default, SQLite does not interrupt queries. To restore the default
* behavior, send 0 as the +ms+ parameter.
*/
static VALUE
set_statement_timeout(VALUE self, VALUE milliseconds)
{
sqlite3RubyPtr ctx;
TypedData_Get_Struct(self, sqlite3Ruby, &database_type, ctx);

ctx->stmt_timeout = NUM2INT(milliseconds);
int n = NUM2INT(milliseconds) == 0 ? -1 : 1000;

sqlite3_progress_handler(ctx->db, n, rb_sqlite3_statement_timeout, (void *)ctx);

return self;
}

/* call-seq: last_insert_row_id
*
* Obtains the unique row ID of the last row to be inserted by this Database
Expand Down Expand Up @@ -875,6 +913,9 @@ init_sqlite3_database(void)
rb_define_method(cSqlite3Database, "authorizer=", set_authorizer, 1);
rb_define_method(cSqlite3Database, "busy_handler", busy_handler, -1);
rb_define_method(cSqlite3Database, "busy_timeout=", set_busy_timeout, 1);
#ifndef SQLITE_OMIT_PROGRESS_CALLBACK
rb_define_method(cSqlite3Database, "statement_timeout=", set_statement_timeout, 1);
#endif
rb_define_method(cSqlite3Database, "extended_result_codes=", set_extended_result_codes, 1);
rb_define_method(cSqlite3Database, "transaction_active?", transaction_active_p, 0);
rb_define_private_method(cSqlite3Database, "exec_batch", exec_batch, 2);
Expand Down
2 changes: 2 additions & 0 deletions ext/sqlite3/database.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
struct _sqlite3Ruby {
sqlite3 *db;
VALUE busy_handler;
int stmt_timeout;
struct timespec stmt_deadline;
};

typedef struct _sqlite3Ruby sqlite3Ruby;
Expand Down
1 change: 1 addition & 0 deletions ext/sqlite3/sqlite3_ruby.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ extern VALUE cSqlite3Blob;
#include <statement.h>
#include <exception.h>
#include <backup.h>
#include <timespec.h>

int bignum_to_int64(VALUE big, sqlite3_int64 *result);

Expand Down
1 change: 1 addition & 0 deletions ext/sqlite3/statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ prepare(VALUE self, VALUE db, VALUE sql)
);

CHECK(db_ctx->db, status);
timespecclear(&db_ctx->stmt_deadline);

return rb_str_new2(tail);
}
Expand Down
20 changes: 20 additions & 0 deletions ext/sqlite3/timespec.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#define timespecclear(tsp) (tsp)->tv_sec = (tsp)->tv_nsec = 0
#define timespecisset(tsp) ((tsp)->tv_sec || (tsp)->tv_nsec)
#define timespecisvalid(tsp) \
((tsp)->tv_nsec >= 0 && (tsp)->tv_nsec < 1000000000L)
#define timespeccmp(tsp, usp, cmp) \
(((tsp)->tv_sec == (usp)->tv_sec) ? \
((tsp)->tv_nsec cmp (usp)->tv_nsec) : \
((tsp)->tv_sec cmp (usp)->tv_sec))
#define timespecsub(tsp, usp, vsp) \
do { \
(vsp)->tv_sec = (tsp)->tv_sec - (usp)->tv_sec; \
(vsp)->tv_nsec = (tsp)->tv_nsec - (usp)->tv_nsec; \
if ((vsp)->tv_nsec < 0) { \
(vsp)->tv_sec--; \
(vsp)->tv_nsec += 1000000000L; \
} \
} while (0)
#define timespecafter(tsp, usp) \
(((tsp)->tv_sec > (usp)->tv_sec) || \
((tsp)->tv_sec == (usp)->tv_sec && (tsp)->tv_nsec > (usp)->tv_nsec))
Copy link
Member

Choose a reason for hiding this comment

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

Isn't timespec.h available on some platforms? I thought we'd conditionally define this stuff depending on platform availability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH, I wasn't sure how best to handle this. Is there a single header file I can try to include that will have these methods defined? How do I conditionally include a header file by whether or not that file exists? And then, do I need to ensure that all methods are present? And timespecafter won't be defined, because that is one I wrote myself to make the logic as direct and explicit as possible in the database file. I considered conditionally defining the macros, but I wasn't certain how to do that, or if I would need to include some header file to try and get the system's macros, if they exist, into the project.

If you have any guidance on how you want to conditionally define this stuff, I'm happy to give it a try. I ended up just deciding that it wasn't much code to bring in, it is pretty clear what it does, and if we own that code, we can guarantee behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

You use have_header in extconf.rb which then define HAVE_... which you can check with a # ifdef

https://ruby-doc.org/stdlib-2.5.1/libdoc/mkmf/rdoc/MakeMakefile.html#method-i-have_header

1 change: 1 addition & 0 deletions lib/sqlite3/database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def initialize file, options = {}, zvfs = nil
@authorizer = nil
@encoding = nil
@busy_handler = nil
@progress_handler = nil
@collations = {}
@functions = {}
@results_as_hash = options[:results_as_hash]
Expand Down
1 change: 1 addition & 0 deletions sqlite3.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Gem::Specification.new do |s|
"ext/sqlite3/sqlite3_ruby.h",
"ext/sqlite3/statement.c",
"ext/sqlite3/statement.h",
"ext/sqlite3/timespec.h",
"lib/sqlite3.rb",
"lib/sqlite3/constants.rb",
"lib/sqlite3/database.rb",
Expand Down
16 changes: 16 additions & 0 deletions test/test_integration_statement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,20 @@ def test_committing_tx_with_statement_active
end
assert called
end

def test_long_running_statements_get_interrupted_when_statement_timeout_set
@db.statement_timeout = 10
assert_raises(SQLite3::InterruptException) do
@db.execute <<~SQL
WITH RECURSIVE r(i) AS (
VALUES(0)
UNION ALL
SELECT i FROM r
LIMIT 100000
)
SELECT i FROM r ORDER BY i LIMIT 1;
SQL
end
@db.statement_timeout = 0
end
end