Skip to content

Commit 56d47a6

Browse files
flavorjonesflavorjones
committed
Optimize the statement check for a non-discarded database
This also restores the ability (now tested!) to call Database#close successfully and defer its cleanup until after Statements are closed, which was the promise of #557 and sqlite3_close_v2. Closes #564
1 parent 81ea485 commit 56d47a6

File tree

6 files changed

+37
-29
lines changed

6 files changed

+37
-29
lines changed

‎ext/sqlite3/database.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ discard_db(sqlite3RubyPtr ctx)
4949
}
5050

5151
ctx->db = NULL;
52+
ctx->flags |= SQLITE_DATABASE_DISCARDED;
5253
}
5354

5455
static void

‎ext/sqlite3/database.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33

44
#include <sqlite3_ruby.h>
55

6+
/* if this ever exceeds the highest bit of the SQLITE_OPEN_* bitmap, we should move it to a
7+
* different integer than the `flags` field */
8+
#define SQLITE_DATABASE_DISCARDED 0x80000000
9+
610
struct _sqlite3Ruby {
711
sqlite3 *db;
812
VALUE busy_handler;

‎ext/sqlite3/statement.c

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,12 @@
11
#include <sqlite3_ruby.h>
22

33
#define REQUIRE_OPEN_STMT(_ctxt) \
4-
if(!_ctxt->st) \
4+
if (!_ctxt->st) \
55
rb_raise(rb_path2class("SQLite3::Exception"), "cannot use a closed statement");
66

7-
static void
8-
require_open_db(VALUE stmt_rb)
9-
{
10-
VALUE closed_p = rb_funcall(
11-
rb_iv_get(stmt_rb, "@connection"),
12-
rb_intern("closed?"), 0);
13-
14-
if (RTEST(closed_p)) {
15-
rb_raise(rb_path2class("SQLite3::Exception"),
16-
"cannot use a statement associated with a closed database");
17-
}
18-
}
19-
7+
#define REQUIRE_LIVE_DB(_ctxt) \
8+
if (_ctxt->db->flags & SQLITE_DATABASE_DISCARDED) \
9+
rb_raise(rb_path2class("SQLite3::Exception"), "cannot use a statement associated with a discarded database");
2010

2111
VALUE cSqlite3Statement;
2212

@@ -71,6 +61,11 @@ prepare(VALUE self, VALUE db, VALUE sql)
7161

7262
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
7363

64+
/* Dereferencing a pointer to the database struct will be faster than accessing it through the
65+
* instance variable @connection. The struct pointer is guaranteed to be live because instance
66+
* variable will keep it from being GCed. */
67+
ctx->db = db_ctx;
68+
7469
#ifdef HAVE_SQLITE3_PREPARE_V2
7570
status = sqlite3_prepare_v2(
7671
#else
@@ -135,7 +130,7 @@ step(VALUE self)
135130

136131
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
137132

138-
require_open_db(self);
133+
REQUIRE_LIVE_DB(ctx);
139134
REQUIRE_OPEN_STMT(ctx);
140135

141136
if (ctx->done_p) { return Qnil; }
@@ -232,7 +227,7 @@ bind_param(VALUE self, VALUE key, VALUE value)
232227

233228
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
234229

235-
require_open_db(self);
230+
REQUIRE_LIVE_DB(ctx);
236231
REQUIRE_OPEN_STMT(ctx);
237232

238233
switch (TYPE(key)) {
@@ -326,7 +321,7 @@ reset_bang(VALUE self)
326321

327322
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
328323

329-
require_open_db(self);
324+
REQUIRE_LIVE_DB(ctx);
330325
REQUIRE_OPEN_STMT(ctx);
331326

332327
sqlite3_reset(ctx->st);
@@ -348,7 +343,7 @@ clear_bindings_bang(VALUE self)
348343

349344
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
350345

351-
require_open_db(self);
346+
REQUIRE_LIVE_DB(ctx);
352347
REQUIRE_OPEN_STMT(ctx);
353348

354349
sqlite3_clear_bindings(ctx->st);
@@ -382,7 +377,7 @@ column_count(VALUE self)
382377
sqlite3StmtRubyPtr ctx;
383378
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
384379

385-
require_open_db(self);
380+
REQUIRE_LIVE_DB(ctx);
386381
REQUIRE_OPEN_STMT(ctx);
387382

388383
return INT2NUM(sqlite3_column_count(ctx->st));
@@ -415,7 +410,7 @@ column_name(VALUE self, VALUE index)
415410

416411
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
417412

418-
require_open_db(self);
413+
REQUIRE_LIVE_DB(ctx);
419414
REQUIRE_OPEN_STMT(ctx);
420415

421416
name = sqlite3_column_name(ctx->st, (int)NUM2INT(index));
@@ -440,7 +435,7 @@ column_decltype(VALUE self, VALUE index)
440435

441436
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
442437

443-
require_open_db(self);
438+
REQUIRE_LIVE_DB(ctx);
444439
REQUIRE_OPEN_STMT(ctx);
445440

446441
name = sqlite3_column_decltype(ctx->st, (int)NUM2INT(index));
@@ -459,7 +454,7 @@ bind_parameter_count(VALUE self)
459454
sqlite3StmtRubyPtr ctx;
460455
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
461456

462-
require_open_db(self);
457+
REQUIRE_LIVE_DB(ctx);
463458
REQUIRE_OPEN_STMT(ctx);
464459

465460
return INT2NUM(sqlite3_bind_parameter_count(ctx->st));
@@ -568,7 +563,7 @@ stats_as_hash(VALUE self)
568563
sqlite3StmtRubyPtr ctx;
569564
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
570565

571-
require_open_db(self);
566+
REQUIRE_LIVE_DB(ctx);
572567
REQUIRE_OPEN_STMT(ctx);
573568

574569
VALUE arg = rb_hash_new();
@@ -587,7 +582,7 @@ stat_for(VALUE self, VALUE key)
587582
sqlite3StmtRubyPtr ctx;
588583
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
589584

590-
require_open_db(self);
585+
REQUIRE_LIVE_DB(ctx);
591586
REQUIRE_OPEN_STMT(ctx);
592587

593588
if (SYMBOL_P(key)) {
@@ -609,7 +604,7 @@ memused(VALUE self)
609604
sqlite3StmtRubyPtr ctx;
610605
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
611606

612-
require_open_db(self);
607+
REQUIRE_LIVE_DB(ctx);
613608
REQUIRE_OPEN_STMT(ctx);
614609

615610
return INT2NUM(sqlite3_stmt_status(ctx->st, SQLITE_STMTSTATUS_MEMUSED, 0));
@@ -628,7 +623,7 @@ database_name(VALUE self, VALUE index)
628623
sqlite3StmtRubyPtr ctx;
629624
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
630625

631-
require_open_db(self);
626+
REQUIRE_LIVE_DB(ctx);
632627
REQUIRE_OPEN_STMT(ctx);
633628

634629
return SQLITE3_UTF8_STR_NEW2(
@@ -647,7 +642,7 @@ get_sql(VALUE self)
647642
sqlite3StmtRubyPtr ctx;
648643
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
649644

650-
require_open_db(self);
645+
REQUIRE_LIVE_DB(ctx);
651646
REQUIRE_OPEN_STMT(ctx);
652647

653648
return rb_obj_freeze(SQLITE3_UTF8_STR_NEW2(sqlite3_sql(ctx->st)));
@@ -667,7 +662,7 @@ get_expanded_sql(VALUE self)
667662

668663
TypedData_Get_Struct(self, sqlite3StmtRuby, &statement_type, ctx);
669664

670-
require_open_db(self);
665+
REQUIRE_LIVE_DB(ctx);
671666
REQUIRE_OPEN_STMT(ctx);
672667

673668
expanded_sql = sqlite3_expanded_sql(ctx->st);

‎ext/sqlite3/statement.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
struct _sqlite3StmtRuby {
77
sqlite3_stmt *st;
8+
sqlite3Ruby *db;
89
int done_p;
910
};
1011

‎test/test_discarding.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ def test_a_discarded_connection_with_statements
160160
db.send(:discard)
161161

162162
e = assert_raises(SQLite3::Exception) { stmt.execute }
163-
assert_match(/cannot use a statement associated with a closed database/, e.message)
163+
assert_match(/cannot use a statement associated with a discarded database/, e.message)
164164

165165
assert_nothing_raised { stmt.close }
166166
assert_predicate(stmt, :closed?)

‎test/test_statement.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,13 @@ def test_new_closed_handle
135135
end
136136
end
137137

138+
def test_closed_db_behavior
139+
@db.close
140+
result = nil
141+
assert_nothing_raised { result = @stmt.execute }
142+
refute_nil result
143+
end
144+
138145
def test_new_with_remainder
139146
stmt = SQLite3::Statement.new(@db, "select 'foo';bar")
140147
assert_equal "bar", stmt.remainder

0 commit comments

Comments
 (0)