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

create_function: impossible to call set_error #164

Closed
lionelperrin opened this issue Sep 24, 2015 · 0 comments · Fixed by #510
Closed

create_function: impossible to call set_error #164

lionelperrin opened this issue Sep 24, 2015 · 0 comments · Fixed by #510
Labels
Milestone

Comments

@lionelperrin
Copy link

FunctionProxy#set_error is implemented like this:

class FunctionProxy
      attr_accessor :result

      # Create a new FunctionProxy that encapsulates the given +func+ object.
      # If context is non-nil, the functions context will be set to that. If
      # it is non-nil, it must quack like a Hash. If it is nil, then none of
      # the context functions will be available.
      def initialize
        @result   = nil
        @context  = {}
      end

      # Set the result of the function to the given error message.
      # The function will then return that error.
      def set_error( error )
        @driver.result_error( @func, error.to_s, -1 )
      end
...

it looks that the instance variable @driver is never set, which causes the following error:
database.rb:559:inset_error': undefined method result_error' for nil:NilClass (NoMethodError)

rangeroob pushed a commit to rangeroob/sqlite3-ruby-static that referenced this issue Apr 11, 2020
Added tests to demonstrate three issues with the way sqlite3-ruby
currently handles aggregate functions:

(1) Defining a second aggregate function and using both results in
memory violation. The problem is that `rb_iv_set(self, "@agregator",
aggregator);`   (database.c:399) just overwrites the last reference to
the first  AggregateHandler, but SQLite still holds a pointer to it and
will follow   the pointer if the first aggregate function is used in a
query.
  The most straight-forward fix is to insert the aggregator in a ruby
array and never release it -- similar to how its done for functions.

(2) Using the same aggregate function twice in a query mixes up values
from both  columns. For example:
  `SELECT MYAGGREG(a), MYAGGREG(b) FROM foo;`
  Leads to: Myaggreg.step(a1); Myaggreg.step(b1); Myaggreg.step(a2);
  Myaggreg.step(b2); ... ; Myaggreg.finalize(), Myaggreg.finalize()
  The SQLite API expects the caller to differentiate between these
chains  of invocation via `sqlite3_aggregate_context()`, but current
sqlite3-ruby  does not account for that.
  #44 has been a work around for this in the special case that the first
  aggregation is finalized before the second started (separate queries).
  sparklemotion#161 does the analog for the function Proxy.

(3) Documentation implies that library users shall explicitly set the
arity of the function. Consequently the library user is likely to
expect that this is passed down to SQLite, so he may define multiple
functions with the same name but different arity and SQLite to use the
most appropriate one  (documented sqlite feature). Unfortunately,
sqlite3-ruby does not pass the arity to SQLite, which is surprising
given that different arity-values  are accounted for in the C code. The
problem is that sqlite3-ruby does not call the AggregateHandlers
"arity" method but instead queries the arity of
  the "step" method from Ruby (sqlite3_obj_method_arity). Confusingly,
this  is not the "step" provided by the library user, but the "step" of
the  Proxy Wrapper classes introduced by Database.create_aggregate or
Database.create_aggregate_handler. Both of these are just
`step(*args)`,  so the arity reported to SQLite is always -1.

Things not addressed:
  - sparklemotion#164 impossible to call FunctionProxy.set_error (and
FunctionProxy.count).  because @driver does not exist (anymore)
  - Database.create_aggregate and the tests contain a 'text_rep' field,
that  is never passed down to SQLite. It is not advertised in the
documentation, though.

(cherry picked from commit ae89dea)
@tenderlove tenderlove added this to the 2.0.0 milestone Jan 24, 2024
@tenderlove tenderlove added the bug label Feb 2, 2024
alexcwatt added a commit to alexcwatt/sqlite3-ruby that referenced this issue Mar 1, 2024
This code depends on a SQLite API which is deprecated,
sqlite3_aggregate_count. In addition, the present implementation
is broken (@driver is always nil). Related: sparklemotion#164.
flavorjones added a commit that referenced this issue Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants