SQLite glue segfault in 7.7

[Bill Welliver]

I just noticed that the SQLite module in 7.7 will segfault if you run  
a query that returns zero rows (as opposed to a query that throws an  
error). Since it uses some array aggregation magic i don't fully  
understand, I thought I'd leave it to someone else. Seems like a  
pretty major bug, though:

 > object s= Sql.Sql("sqlite:///tmp/speedydelivery.sqlite3");
 > s->query("select * from lists where name=0");

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x0000000c
0x006d276f in f_SQLite_query (args=1) at sqlite.cmod:326
 

[Bill Welliver]

While I'm at reporting problems with the sqlite glue, I noticed that  
the query() method for SQLite returns a non-standard result format:

 > s->query("select * from lists");
(3) Result: ({ /* 1 element */
                 ({ /* 4 elements */
                     "1",
                     "pdt-devel",
                     "a development discussion list for the pdt (pike  
eclipse plugin) project.",
                     "2008-07-22 19:01:09"
                 })
             })


shouldn't that return an array of mappings indexed on field name?

Bill

Try replacing

    if (Pike_sp[0].u.array->size == 0) {

with

    if (Pike_sp[-1].u.array->size == 0) {

[Stephen R. van den Berg]

H. William Welliver III wrote:
>While I'm at reporting problems with the sqlite glue, I noticed that  
>the query() method for SQLite returns a non-standard result format:

>shouldn't that return an array of mappings indexed on field name?

It should not implement query, but big_query.  query is going to be
provided by the Sql.Sql class.
--
Sincerely,
           Stephen R. van den Berg.

"Always remember that you are unique.  Just like everyone else."

[Bill Welliver]

> It should not implement query, but big_query.  query is going to be
> provided by the Sql.Sql class.

Actually, big_query is much more difficult to implement and not all
databases have facilities to work in that manner. There's a helper that
allows a big_query to be generated from query(). There was a conversation
about it a year or two when I wrote my SQLite glue.

Bill

[Bill Welliver]

Yes, thanks... it occurred to me that might be the solution after I  
wrote. I didn't follow up when I realized that the SQLite database  
module is so different in behavior from the other database modules  
that I decided to give up and continue using my own SQLite glue.

As a note, I'd suggest that a big flag be put up in the notes about  
the bundled SQLite module. It returns array(array) instead of array
(mapping) from query(), such that it's not possible to use it with  
any code normally written for databases in Pike. That's certainly  
fixable, but I don't think it can be fixed in time for this stable  
release. Perhaps it shouldn't be included/disabled?

Bill

On Jul 22, 2008, at 7:20 PM, Martin Stjernholm, Roxen IS @ Pike  
developers forum wrote:

> Try replacing
>
>     if (Pike_sp[0].u.array->size == 0) {
>
> with
>
>     if (Pike_sp[-1].u.array->size == 0) {

> As a note, I'd suggest that a big flag be put up in the notes about
> the bundled SQLite module. It returns array(array) instead of array
> (mapping) from query(), such that it's not possible to use it with  
> any code normally written for databases in Pike. /.../

But that's not new, is it? Certainly it should be fixed (or possibly
deprecated and replaced with a rewritten module), but that'd require
compat goo since there might be code that contains kludges to cope
with this strange behavior.

A prominent @note in the refdoc string for that query function sounds
like a good idea.

> Not sure how you define "new"... I don't believe it's been available  
> before 7.7. I never looked at it because I have my own, but figured  
> it might be worthwhile to start using the one bundled.

A bit late reply, but anyway: If SQLite is new in 7.7 I definitely
think this should be fixed.

[Bill Welliver]

I agree. I'll try to look at it this evening; it shouldn't be too hard to
do; the module's only 500 lines long to begin with!

Bill

On Thu, 31 Jul 2008, Martin Stjernholm, Roxen IS @ Pike developers forum wrote:

>> Not sure how you define "new"... I don't believe it's been available
>> before 7.7. I never looked at it because I have my own, but figured
>> it might be worthwhile to start using the one bundled.
>
> A bit late reply, but anyway: If SQLite is new in 7.7 I definitely
> think this should be fixed.
>

[Bill Welliver]

I spent a little time this evening and got the most critical parts of  
the SQLite module working like other database modules (imho).

Any chance we can get sqlite support compiled into the Windows build?  
It'd sure be nice to have out of box support for a db on Windows, as  
it's such a pain to recompile.

Bill

On Jul 31, 2008, at 9:45 AM, Martin Stjernholm, Roxen IS @ Pike  
developers forum wrote:

>> Not sure how you define "new"... I don't believe it's been available
>> before 7.7. I never looked at it because I have my own, but figured
>> it might be worthwhile to start using the one bundled.
>
> A bit late reply, but anyway: If SQLite is new in 7.7 I definitely
> think this should be fixed.

I suppose.

My compilation environment now includes SQLite 3.6.0.