Skip to content

Commit ae89dea

Browse files
committed
tests to demonstrate issues with aggregate function handling
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.
1 parent 030d029 commit ae89dea

File tree

1 file changed

+148
-0
lines changed

1 file changed

+148
-0
lines changed

test/test_integration_aggregate.rb

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,127 @@ def test_create_aggregate_with_block
4747
assert_equal 6, value
4848
end
4949

50+
def test_create_aggregate_with_the_same_function_twice_in_a_query
51+
@db.create_aggregate( "accumulate", 1 ) do
52+
step do |ctx,a|
53+
ctx[:sum] ||= 0
54+
ctx[:sum] += a.to_i
55+
end
56+
57+
finalize { |ctx| ctx.result = ctx[:sum] }
58+
end
59+
60+
values = @db.get_first_row( "select accumulate(a), accumulate(c) from foo" )
61+
assert_equal 6, values[0]
62+
assert_equal 33, values[1]
63+
end
64+
65+
def test_create_aggregate_with_two_different_functions
66+
@db.create_aggregate( "accumulate", 1 ) do
67+
step do |ctx,a|
68+
ctx[:sum] ||= 0
69+
ctx[:sum] += a.to_i
70+
end
71+
72+
finalize { |ctx| ctx.result = ctx[:sum] }
73+
end
74+
75+
@db.create_aggregate( "multiply", 1 ) do
76+
step do |ctx,a|
77+
ctx[:sum] ||= 1
78+
ctx[:sum] *= a.to_i
79+
end
80+
81+
finalize { |ctx| ctx.result = ctx[:sum] }
82+
end
83+
84+
# This is likely to crash, as ruby-sqlite overwrites its only reference
85+
# to the "accumulate" handler with the "multiply" handler, although
86+
# SQLite still holds a pointer which it follows on next select.
87+
#GC.start
88+
89+
values = @db.get_first_row( "select accumulate(a), multiply(c) from foo" )
90+
assert_equal 6, values[0]
91+
assert_equal 1320, values[1]
92+
93+
value = @db.get_first_value( "select accumulate(c) from foo")
94+
assert_equal 33, value
95+
96+
value = @db.get_first_value( "select multiply(a) from foo")
97+
assert_equal 6, value
98+
end
99+
100+
def test_create_aggregate_overwrite_function
101+
@db.create_aggregate( "accumulate", 1 ) do
102+
step do |ctx,a|
103+
ctx[:sum] ||= 0
104+
ctx[:sum] += a.to_i
105+
end
106+
107+
finalize { |ctx| ctx.result = ctx[:sum] }
108+
end
109+
110+
value = @db.get_first_value( "select accumulate(c) from foo")
111+
assert_equal 33, value
112+
113+
GC.start
114+
115+
@db.create_aggregate( "accumulate", 1 ) do
116+
step do |ctx,a|
117+
ctx[:sum] ||= 1
118+
ctx[:sum] *= a.to_i
119+
end
120+
121+
finalize { |ctx| ctx.result = ctx[:sum] }
122+
end
123+
124+
value = @db.get_first_value( "select accumulate(c) from foo")
125+
assert_equal 1320, value
126+
end
127+
128+
def test_create_aggregate_overwrite_function_with_different_arity
129+
@db.create_aggregate( "accumulate", -1 ) do
130+
step do |ctx,*args|
131+
ctx[:sum] ||= 0
132+
args.each { |a| ctx[:sum] += a.to_i }
133+
end
134+
135+
finalize { |ctx| ctx.result = ctx[:sum] }
136+
end
137+
138+
@db.create_aggregate( "accumulate", 2 ) do
139+
step do |ctx,a,b|
140+
ctx[:sum] ||= 1
141+
ctx[:sum] *= (a.to_i + b.to_i)
142+
end
143+
144+
finalize { |ctx| ctx.result = ctx[:sum] }
145+
end
146+
147+
GC.start
148+
149+
values = @db.get_first_row( "select accumulate(c), accumulate(a,c) from foo")
150+
assert_equal 33, values[0]
151+
assert_equal 39, values[1]
152+
end
153+
154+
class CustomException < Exception
155+
end
156+
157+
def test_create_aggregate_with_exception
158+
@db.create_aggregate( "raiseexception", 1 ) do
159+
step do |ctx,a|
160+
raise CustomException.new( "bogus aggregate handler" )
161+
end
162+
163+
finalize { |ctx| ctx.result = 42 }
164+
end
165+
166+
assert_raise CustomException do
167+
@db.get_first_value( "select raiseexception(a) from foo")
168+
end
169+
end
170+
50171
def test_create_aggregate_with_no_data
51172
@db.create_aggregate( "accumulate", 1 ) do
52173
step do |ctx,a|
@@ -90,6 +211,33 @@ def test_aggregate_initialized_twice
90211
assert_equal 2, initialized
91212
end
92213

214+
def test_create_aggregate_handler_call_with_wrong_arity
215+
@db.create_aggregate_handler AggregateHandler
216+
217+
assert_raise (SQLite3::SQLException) do
218+
@db.get_first_value( "select multiply(a,c) from foo" )
219+
end
220+
end
221+
222+
class RaiseExceptionAggregateHandler
223+
class << self
224+
def arity; 1; end
225+
def text_rep; SQLite3::Constants::TextRep::ANY; end
226+
def name; "raiseexception"; end
227+
end
228+
def step(ctx, a)
229+
raise CustomException.new( "bogus aggregate handler" )
230+
end
231+
def finalize(ctx); ctx.result = nil; end
232+
end
233+
234+
def test_create_aggregate_handler_with_exception
235+
@db.create_aggregate_handler RaiseExceptionAggregateHandler
236+
assert_raise CustomException do
237+
@db.get_first_value( "select raiseexception(a) from foo")
238+
end
239+
end
240+
93241
def test_create_aggregate_handler
94242
@db.create_aggregate_handler AggregateHandler
95243
value = @db.get_first_value( "select multiply(a) from foo" )

0 commit comments

Comments
 (0)