diff --git a/Manifest.txt b/Manifest.txt index 696c7f3e..01da3677 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -9,6 +9,8 @@ Manifest.txt README.rdoc Rakefile appveyor.yml +ext/sqlite3/aggregator.c +ext/sqlite3/aggregator.h ext/sqlite3/backup.c ext/sqlite3/backup.h ext/sqlite3/database.c @@ -47,6 +49,7 @@ test/test_database_readwrite.rb test/test_deprecated.rb test/test_encoding.rb test/test_integration.rb +test/test_integration_aggregate.rb test/test_integration_open_close.rb test/test_integration_pending.rb test/test_integration_resultset.rb diff --git a/ext/sqlite3/aggregator.c b/ext/sqlite3/aggregator.c new file mode 100644 index 00000000..2a1ebf71 --- /dev/null +++ b/ext/sqlite3/aggregator.c @@ -0,0 +1,282 @@ +#include +#include + +/* wraps a factory "handler" class. The "-aggregators" instance variable of + * the SQLite3::Database holds an array of all AggrogatorWrappers. + * + * An AggregatorWrapper holds the following instance variables: + * -handler_klass: the handler that creates the instances. + * -instances: array of all the cAggregatorInstance objects currently + * in-flight for this aggregator. */ +static VALUE cAggregatorWrapper; + +/* wraps a intance of the "handler" class. Loses its reference at the end of + * the xFinal callback. + * + * An AggregatorInstance holds the following instnace variables: + * -handler_instance: the instance to call `step` and `finalize` on. + * -exc_status: status returned by rb_protect. + * != 0 if an exception occurred. If an exception occured + * `step` and `finalize` won't be called any more. */ +static VALUE cAggregatorInstance; + +typedef struct rb_sqlite3_protected_funcall_args { + VALUE self; + ID method; + int argc; + VALUE *params; +} protected_funcall_args_t; + +/* why isn't there something like this in the ruby API? */ +static VALUE +rb_sqlite3_protected_funcall_body(VALUE protected_funcall_args_ptr) +{ + protected_funcall_args_t *args = + (protected_funcall_args_t*)protected_funcall_args_ptr; + + return rb_funcall2(args->self, args->method, args->argc, args->params); +} + +static VALUE +rb_sqlite3_protected_funcall(VALUE self, ID method, int argc, VALUE *params, + int* exc_status) +{ + protected_funcall_args_t args = { + .self = self, .method = method, .argc = argc, .params = params + }; + return rb_protect(rb_sqlite3_protected_funcall_body, (VALUE)(&args), exc_status); +} + +/* called in rb_sqlite3_aggregator_step and rb_sqlite3_aggregator_final. It + * checks if the exection context already has an associated instance. If it + * has one, it returns it. If there is no instance yet, it creates one and + * associates it with the context. */ +static VALUE +rb_sqlite3_aggregate_instance(sqlite3_context *ctx) +{ + VALUE aw = (VALUE) sqlite3_user_data(ctx); + VALUE handler_klass = rb_iv_get(aw, "-handler_klass"); + VALUE inst; + VALUE *inst_ptr = sqlite3_aggregate_context(ctx, (int)sizeof(VALUE)); + + if (!inst_ptr) { + rb_fatal("SQLite is out-of-merory"); + } + + inst = *inst_ptr; + + if (inst == Qfalse) { /* Qfalse == 0 */ + VALUE instances = rb_iv_get(aw, "-instances"); + int exc_status; + + inst = rb_class_new_instance(0, NULL, cAggregatorInstance); + rb_iv_set(inst, "-handler_instance", rb_sqlite3_protected_funcall( + handler_klass, rb_intern("new"), 0, NULL, &exc_status)); + rb_iv_set(inst, "-exc_status", INT2NUM(exc_status)); + + rb_ary_push(instances, inst); + + *inst_ptr = inst; + } + + if (inst == Qnil) { + rb_fatal("SQLite called us back on an already destroyed aggregate instance"); + } + + return inst; +} + +/* called by rb_sqlite3_aggregator_final. Unlinks and frees the + * aggregator_instance_t, so the handler_instance won't be marked any more + * and Ruby's GC may free it. */ +static void +rb_sqlite3_aggregate_instance_destroy(sqlite3_context *ctx) +{ + VALUE aw = (VALUE) sqlite3_user_data(ctx); + VALUE instances = rb_iv_get(aw, "-instances"); + VALUE *inst_ptr = sqlite3_aggregate_context(ctx, 0); + VALUE inst; + + if (!inst_ptr || (inst = *inst_ptr)) { + return; + } + + if (inst == Qnil) { + rb_fatal("attempt to destroy aggregate instance twice"); + } + + rb_iv_set(inst, "-handler_instance", Qnil); // may catch use-after-free + if (rb_ary_delete(instances, inst) == Qnil) { + rb_fatal("must be in instances at that point"); + } + + *inst_ptr = Qnil; +} + +static void +rb_sqlite3_aggregator_step(sqlite3_context * ctx, int argc, sqlite3_value **argv) +{ + VALUE inst = rb_sqlite3_aggregate_instance(ctx); + VALUE handler_instance = rb_iv_get(inst, "-handler_instance"); + VALUE * params = NULL; + VALUE one_param; + int exc_status = NUM2INT(rb_iv_get(inst, "-exc_status")); + int i; + + if (exc_status) { + return; + } + + if (argc == 1) { + one_param = sqlite3val2rb(argv[i]); + params = &one_param; + } + if (argc > 1) { + params = xcalloc((size_t)argc, sizeof(VALUE)); + for(i = 0; i < argc; i++) { + params[i] = sqlite3val2rb(argv[i]); + } + } + rb_sqlite3_protected_funcall( + handler_instance, rb_intern("step"), argc, params, &exc_status); + if (argc > 1) { + xfree(params); + } + + rb_iv_set(inst, "-exc_status", INT2NUM(exc_status)); +} + +/* we assume that this function is only called once per execution context */ +static void +rb_sqlite3_aggregator_final(sqlite3_context * ctx) +{ + VALUE inst = rb_sqlite3_aggregate_instance(ctx); + VALUE handler_instance = rb_iv_get(inst, "-handler_instance"); + int exc_status = NUM2INT(rb_iv_get(inst, "-exc_status")); + + if (!exc_status) { + VALUE result = rb_sqlite3_protected_funcall( + handler_instance, rb_intern("finalize"), 0, NULL, &exc_status); + if (!exc_status) { + set_sqlite3_func_result(ctx, result); + } + } + + if (exc_status) { + /* the user should never see this, as Statement.step() will pick up the + * outstanding exception and raise it instead of generating a new one + * for SQLITE_ERROR with message "Ruby Exception occured" */ + sqlite3_result_error(ctx, "Ruby Exception occured", -1); + } + + rb_sqlite3_aggregate_instance_destroy(ctx); +} + +/* call-seq: define_aggregator2(aggregator) + * + * Define an aggregrate function according to a factory object (the "handler") + * that knows how to obtain to all the information. The handler must provide + * the following class methods: + * + * +arity+:: corresponds to the +arity+ parameter of #create_aggregate. This + * message is optional, and if the handler does not respond to it, + * the function will have an arity of -1. + * +name+:: this is the name of the function. The handler _must_ implement + * this message. + * +new+:: this must be implemented by the handler. It should return a new + * instance of the object that will handle a specific invocation of + * the function. + * + * The handler instance (the object returned by the +new+ message, described + * above), must respond to the following messages: + * + * +step+:: this is the method that will be called for each step of the + * aggregate function's evaluation. It should take parameters according + * to the *arity* definition. + * +finalize+:: this is the method that will be called to finalize the + * aggregate function's evaluation. It should not take arguments. + * + * Note the difference between this function and #create_aggregate_handler + * is that no FunctionProxy ("ctx") object is involved. This manifests in two + * ways: The return value of the aggregate function is the return value of + * +finalize+ and neither +step+ nor +finalize+ take an additional "ctx" + * parameter. + */ +VALUE +rb_sqlite3_define_aggregator2(VALUE self, VALUE aggregator) +{ + /* define_aggregator is added as a method to SQLite3::Database in database.c */ + sqlite3RubyPtr ctx; + int arity, status; + VALUE ruby_name; + const char *name; + VALUE aw; + VALUE aggregators; + + Data_Get_Struct(self, sqlite3Ruby, ctx); + if (!ctx->db) { + rb_raise(rb_path2class("SQLite3::Exception"), "cannot use a closed database"); + } + + /* aggregator is typically a class and testing for :name or :new in class + * is a bit pointless */ + + ruby_name = rb_funcall(aggregator, rb_intern("name"), 0); + + if (rb_respond_to(aggregator, rb_intern("arity"))) { + VALUE ruby_arity = rb_funcall(aggregator, rb_intern("arity"), 0); + arity = NUM2INT(ruby_arity); + } else { + arity = -1; + } + + if (arity < -1 || arity > 127) { +#ifdef PRIsVALUE + rb_raise(rb_eArgError, "%"PRIsVALUE" arity=%d out of range -1..127", + self, arity); +#else + rb_raise(rb_eArgError, "Aggregator arity=%d out of range -1..127", arity); +#endif + } + + if (!rb_ivar_defined(self, rb_intern("-aggregators"))) { + rb_iv_set(self, "-aggregators", rb_ary_new()); + } + aggregators = rb_iv_get(self, "-aggregators"); + + name = StringValueCStr(ruby_name); + + aw = rb_class_new_instance(0, NULL, cAggregatorWrapper); + rb_iv_set(aw, "-handler_klass", aggregator); + rb_iv_set(aw, "-instances", rb_ary_new()); + + status = sqlite3_create_function( + ctx->db, + name, + arity, + SQLITE_UTF8, + (void*)aw, + NULL, + rb_sqlite3_aggregator_step, + rb_sqlite3_aggregator_final + ); + + if (status != SQLITE_OK) { + rb_sqlite3_raise(ctx->db, status); + return self; // just in case rb_sqlite3_raise returns. + } + + rb_ary_push(aggregators, aw); + + return self; +} + +void +rb_sqlite3_aggregator_init(void) +{ + rb_gc_register_address(&cAggregatorWrapper); + rb_gc_register_address(&cAggregatorInstance); + /* rb_class_new generatos class with undefined allocator in ruby 1.9 */ + cAggregatorWrapper = rb_funcall(rb_cClass, rb_intern("new"), 0); + cAggregatorInstance = rb_funcall(rb_cClass, rb_intern("new"), 0); +} diff --git a/ext/sqlite3/aggregator.h b/ext/sqlite3/aggregator.h new file mode 100644 index 00000000..e5b32e3d --- /dev/null +++ b/ext/sqlite3/aggregator.h @@ -0,0 +1,12 @@ +#ifndef SQLITE3_AGGREGATOR_RUBY +#define SQLITE3_AGGREGATOR_RUBY + +#include + +VALUE +rb_sqlite3_define_aggregator2(VALUE self, VALUE aggregator); + +void +rb_sqlite3_aggregator_init(void); + +#endif diff --git a/ext/sqlite3/database.c b/ext/sqlite3/database.c index 67d34b57..a60c9f10 100644 --- a/ext/sqlite3/database.c +++ b/ext/sqlite3/database.c @@ -1,4 +1,5 @@ #include +#include #define REQUIRE_OPEN_DB(_ctxt) \ if(!_ctxt->db) \ @@ -73,6 +74,8 @@ static VALUE sqlite3_rb_close(VALUE self) ctx->db = NULL; + rb_iv_set(self, "-aggregators", Qnil); + return self; } @@ -200,7 +203,7 @@ static VALUE last_insert_row_id(VALUE self) return LL2NUM(sqlite3_last_insert_rowid(ctx->db)); } -static VALUE sqlite3val2rb(sqlite3_value * val) +VALUE sqlite3val2rb(sqlite3_value * val) { switch(sqlite3_value_type(val)) { case SQLITE_INTEGER: @@ -230,7 +233,7 @@ static VALUE sqlite3val2rb(sqlite3_value * val) } } -static void set_sqlite3_func_result(sqlite3_context * ctx, VALUE result) +void set_sqlite3_func_result(sqlite3_context * ctx, VALUE result) { switch(TYPE(result)) { case T_NIL: @@ -347,72 +350,6 @@ static VALUE define_function(VALUE self, VALUE name) return define_function_with_flags(self, name, SQLITE_UTF8); } -static int sqlite3_obj_method_arity(VALUE obj, ID id) -{ - VALUE method = rb_funcall(obj, rb_intern("method"), 1, ID2SYM(id)); - VALUE arity = rb_funcall(method, rb_intern("arity"), 0); - - return (int)NUM2INT(arity); -} - -static void rb_sqlite3_step(sqlite3_context * ctx, int argc, sqlite3_value **argv) -{ - VALUE callable = (VALUE)sqlite3_user_data(ctx); - VALUE * params = NULL; - int i; - - if (argc > 0) { - params = xcalloc((size_t)argc, sizeof(VALUE *)); - for(i = 0; i < argc; i++) { - params[i] = sqlite3val2rb(argv[i]); - } - } - rb_funcall2(callable, rb_intern("step"), argc, params); - xfree(params); -} - -static void rb_sqlite3_final(sqlite3_context * ctx) -{ - VALUE callable = (VALUE)sqlite3_user_data(ctx); - VALUE result = rb_funcall(callable, rb_intern("finalize"), 0); - set_sqlite3_func_result(ctx, result); -} - -/* call-seq: define_aggregator(name, aggregator) - * - * Define an aggregate function named +name+ using the object +aggregator+. - * +aggregator+ must respond to +step+ and +finalize+. +step+ will be called - * with row information and +finalize+ must return the return value for the - * aggregator function. - */ -static VALUE define_aggregator(VALUE self, VALUE name, VALUE aggregator) -{ - sqlite3RubyPtr ctx; - int arity, status; - - Data_Get_Struct(self, sqlite3Ruby, ctx); - REQUIRE_OPEN_DB(ctx); - - arity = sqlite3_obj_method_arity(aggregator, rb_intern("step")); - - status = sqlite3_create_function( - ctx->db, - StringValuePtr(name), - arity, - SQLITE_UTF8, - (void *)aggregator, - NULL, - rb_sqlite3_step, - rb_sqlite3_final - ); - - rb_iv_set(self, "@agregator", aggregator); - - CHECK(ctx->db, status); - - return self; -} - /* call-seq: interrupt * * Interrupts the currently executing operation, causing it to abort. @@ -856,7 +793,9 @@ void init_sqlite3_database() rb_define_method(cSqlite3Database, "last_insert_row_id", last_insert_row_id, 0); rb_define_method(cSqlite3Database, "define_function", define_function, 1); rb_define_method(cSqlite3Database, "define_function_with_flags", define_function_with_flags, 2); - rb_define_method(cSqlite3Database, "define_aggregator", define_aggregator, 2); + /* public "define_aggregator" is now a shim around define_aggregator2 + * implemented in Ruby */ + rb_define_private_method(cSqlite3Database, "define_aggregator2", rb_sqlite3_define_aggregator2, 1); rb_define_method(cSqlite3Database, "interrupt", interrupt, 0); rb_define_method(cSqlite3Database, "errmsg", errmsg, 0); rb_define_method(cSqlite3Database, "errcode", errcode_, 0); @@ -879,4 +818,6 @@ void init_sqlite3_database() #endif rb_define_method(cSqlite3Database, "encoding", db_encoding, 0); + + rb_sqlite3_aggregator_init(); } diff --git a/ext/sqlite3/database.h b/ext/sqlite3/database.h index 63e5e961..e0877894 100644 --- a/ext/sqlite3/database.h +++ b/ext/sqlite3/database.h @@ -11,5 +11,7 @@ typedef struct _sqlite3Ruby sqlite3Ruby; typedef sqlite3Ruby * sqlite3RubyPtr; void init_sqlite3_database(); +void set_sqlite3_func_result(sqlite3_context * ctx, VALUE result); +VALUE sqlite3val2rb(sqlite3_value * val); #endif diff --git a/ext/sqlite3/statement.c b/ext/sqlite3/statement.c index 0e5ef85d..2dab7a79 100644 --- a/ext/sqlite3/statement.c +++ b/ext/sqlite3/statement.c @@ -125,6 +125,15 @@ static VALUE step(VALUE self) stmt = ctx->st; value = sqlite3_step(stmt); + if (rb_errinfo() != Qnil) { + /* some user defined function was invoked as a callback during step and + * it raised an exception that has been suppressed until step returns. + * Now re-raise it. */ + VALUE exception = rb_errinfo(); + rb_set_errinfo(Qnil); + rb_exc_raise(exception); + } + length = sqlite3_column_count(stmt); list = rb_ary_new2((long)length); diff --git a/lib/sqlite3/database.rb b/lib/sqlite3/database.rb index 304b10f0..2657a1f5 100644 --- a/lib/sqlite3/database.rb +++ b/lib/sqlite3/database.rb @@ -440,42 +440,52 @@ def create_function name, arity, text_rep=Constants::TextRep::UTF8, &block def create_aggregate( name, arity, step=nil, finalize=nil, text_rep=Constants::TextRep::ANY, &block ) - factory = Class.new do + proxy = Class.new do def self.step( &block ) - define_method(:step, &block) + define_method(:step_with_ctx, &block) end def self.finalize( &block ) - define_method(:finalize, &block) + define_method(:finalize_with_ctx, &block) end end if block_given? - factory.instance_eval(&block) + proxy.instance_eval(&block) else - factory.class_eval do - define_method(:step, step) - define_method(:finalize, finalize) + proxy.class_eval do + define_method(:step_with_ctx, step) + define_method(:finalize_with_ctx, finalize) end end - proxy = factory.new - proxy.extend(Module.new { - attr_accessor :ctx + proxy.class_eval do + # class instance variables + @name = name + @arity = arity + + def self.name + @name + end + + def self.arity + @arity + end + + def initialize + @ctx = FunctionProxy.new + end def step( *args ) - super(@ctx, *args) + step_with_ctx(@ctx, *args) end def finalize - super(@ctx) - result = @ctx.result - @ctx = FunctionProxy.new - result + finalize_with_ctx(@ctx) + @ctx.result end - }) - proxy.ctx = FunctionProxy.new - define_aggregator(name, proxy) + end + define_aggregator2(proxy) end # This is another approach to creating an aggregate function (see @@ -526,29 +536,75 @@ def finalize # db.create_aggregate_handler( LengthsAggregateHandler ) # puts db.get_first_value( "select lengths(name) from A" ) def create_aggregate_handler( handler ) - proxy = Class.new do - def initialize klass - @klass = klass - @fp = FunctionProxy.new + # This is a compatiblity shim so the (basically pointless) FunctionProxy + # "ctx" object is passed as first argument to both step() and finalize(). + # Now its up to the library user whether he prefers to store his + # temporaries as instance varibales or fields in the FunctionProxy. + # The library user still must set the result value with + # FunctionProxy.result= as there is no backwards compatible way to + # change this. + proxy = Class.new(handler) do + def initialize + super + @fp = FunctionProxy.new end def step( *args ) - instance.step(@fp, *args) + super(@fp, *args) end def finalize - instance.finalize @fp - @instance = nil + super(@fp) @fp.result end + end + define_aggregator2(proxy) + self + end + + # Define an aggregate function named +name+ using a object template + # object +aggregator+. +aggregator+ must respond to +step+ and +finalize+. + # +step+ will be called with row information and +finalize+ must return the + # return value for the aggregator function. + # + # _API Change:_ +aggregator+ must also implement +clone+. The provided + # +aggregator+ object will serve as template that is cloned to provide the + # individual instances of the aggregate function. Regular ruby objects + # already provide a suitable +clone+. + # The functions arity is the arity of the +step+ method. + def define_aggregator( name, aggregator ) + # Previously, this has been implemented in C. Now this is just yet + # another compatiblity shim + proxy = Class.new do + @template = aggregator + @name = name + + def self.template + @template + end - private + def self.name + @name + end - def instance - @instance ||= @klass.new + def self.arity + # this is what sqlite3_obj_method_arity did before + @template.method(:step).arity + end + + def initialize + @klass = self.class.template.clone + end + + def step(*args) + @klass.step(*args) + end + + def finalize + @klass.finalize end end - define_aggregator(handler.name, proxy.new(handler)) + define_aggregator2(proxy) self end diff --git a/test/test_integration.rb b/test/test_integration.rb index 2f94691b..d5ea3a53 100644 --- a/test/test_integration.rb +++ b/test/test_integration.rb @@ -499,87 +499,6 @@ def test_create_function assert_match( />>>.*<<