You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bob Rossi <bo...@cox.net> on 2008/05/17 15:07:54 UTC
sqlite3 and postgresql
Hi,
I've been using sqlite3 dbd for some time now. I find that the function
apr_dbd_get_row,
http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
is 1 based. That is, 1 is the first row returned from a query. However,
I just started using postgresql, and I find that apr_dbd_get_row is 0
based. That is, 0 is the first row returned from the query.
Is this expected behavior? Perhaps a bug in my test? The test is,
"create table tbl1(one varchar(10), two smallint);"
"insert into tbl1 values('hello!',10);"
"insert into tbl1 values('goodbye', 20);
"select * from tbl1;"
and then I call apr_dbd_get_row, and compare that the first row returned
from the query matches the "hello!" and the second matches the
"goodbye". With sqlite3 driver, I have to pass in 1 and 2 to
apr_dbd_get_row. With postgresql driver, I have to pass in 0 and 1.
Is this something I should code up on my side?
Thanks,
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
On Sat, 2008-05-17 at 20:43 -0400, Tom Donovan wrote:
> I would be happy to take care of it as soon as I receive commit access. I'm a "new committer", and
> the instructions advised patience - so I expect this usually takes some time to get processed.
I committed something that _may_ work in r657516. The caveat is that we
rely here on both PostgreSQL and MySQL to not blow up in bad ways if
they get rownum of -1 (i.e. if the user of our API passes in row 0). I
did not test this at all, so feel free to do so.
--
Bojan
Re: sqlite3 and postgresql
Posted by Tom Donovan <do...@bellatlantic.net>.
Bojan Smojver wrote:
> On Sat, 2008-05-17 at 18:29 -0500, William A. Rowe, Jr. wrote:
>
>> +1 and I concur with Tom's suggestion w.r.t. a preferred row offset 1.
>
> Tom, do you want to fix this one or do you want me to do it?
>
I would be happy to take care of it as soon as I receive commit access. I'm a "new committer", and
the instructions advised patience - so I expect this usually takes some time to get processed.
-tom-
Re: sqlite3 and postgresql
Posted by Tom Donovan <do...@bellatlantic.net>.
Bob Rossi wrote:
> On Sun, May 18, 2008 at 10:17:34AM +1000, Bojan Smojver wrote:
>> On Sat, 2008-05-17 at 18:29 -0500, William A. Rowe, Jr. wrote:
>>
>>> +1 and I concur with Tom's suggestion w.r.t. a preferred row offset 1.
>> Tom, do you want to fix this one or do you want me to do it?
>
> Aren't you worried about how that will break all the existing programs
> out there using this interface?
>
> Bob Rossi
>
Yes that's a problem - but as it stands existing programs need to change code to change databases.
I don't think this is a candidate for backporting (for the reason you state) - but it makes sense to
document a correct way, and to fix it in the next major APR release.
Otherwise, applications would either work with some dbd drivers but not with others; or else they
must support both 1-based and 0-based row numbers - which implies a code change for most existing
dbd applications (like yours...)
A Google code search turns up few applications which use random-access dbd. Not a guarantee of
course - but an indication that that this won't be disruptive to a large number of apps if we do it
soon.
Surprisingly, there is even an app (a nagios add-on) which tries to switch between 0- and 1-based
row numbering; but it does so only for Oracle vs. non-Oracle, not the general case.
-tom-
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
Quoting Bob Rossi <bo...@cox.net>:
> Aren't you worried about how that will break all the existing programs
> out there using this interface?
This is one of those damned if we fix it, damned if we don't things :-)
That being said, this is clearly a bug, as APU's DBD is supposed to be
database neutral as much a possible (i.e. you should be able to change
the underlying driver and still have most of the stuff just work). For
that reason, it should be fixed.
In terms of strict binary compatibility, 1.2.x most likely cannot be
changed here because we state here
http://apr.apache.org/versioning.html#strategy that for the Patch
Version:
"Changes to the API, to the signatures of public functions, or to the
interpretation of function parameters is not allowed."
AFAICT, Minor Versions do not have this particular requirement, so we
should be OK to fix this in 1.3.x.
PS. We already did similar changes to DBD functions in 1.3.x.
--
Bojan
Re: sqlite3 and postgresql
Posted by Bob Rossi <bo...@cox.net>.
On Sun, May 18, 2008 at 10:17:34AM +1000, Bojan Smojver wrote:
> On Sat, 2008-05-17 at 18:29 -0500, William A. Rowe, Jr. wrote:
>
> > +1 and I concur with Tom's suggestion w.r.t. a preferred row offset 1.
>
> Tom, do you want to fix this one or do you want me to do it?
Aren't you worried about how that will break all the existing programs
out there using this interface?
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
On Sat, 2008-05-17 at 18:29 -0500, William A. Rowe, Jr. wrote:
> +1 and I concur with Tom's suggestion w.r.t. a preferred row offset 1.
Tom, do you want to fix this one or do you want me to do it?
--
Bojan
Re: sqlite3 and postgresql
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Bojan Smojver wrote:
> On Sat, 2008-05-17 at 10:03 -0400, Tom Donovan wrote:
>
>> I think this is a valid bug.
>
> I agree.
+1 and I concur with Tom's suggestion w.r.t. a preferred row offset 1.
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
On Sat, 2008-05-17 at 10:03 -0400, Tom Donovan wrote:
> I think this is a valid bug.
I agree.
--
Bojan
Re: sqlite3 and postgresql
Posted by Tom Donovan <do...@bellatlantic.net>.
Bob Rossi wrote:
> Hi,
>
> I've been using sqlite3 dbd for some time now. I find that the function
> apr_dbd_get_row,
> http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
> is 1 based. That is, 1 is the first row returned from a query. However,
> I just started using postgresql, and I find that apr_dbd_get_row is 0
> based. That is, 0 is the first row returned from the query.
>
> Is this expected behavior? Perhaps a bug in my test? The test is,
The doc doesn't specify, and the current dbd drivers appear to be inconsistent about whether rownum
is 1-based or 0-based for random access:
APR 1.2 drivers
sqlite2 1-based (the driver specifically adjusts it to be 1-based)
sqlite3 1-based (the driver specifically adjusts it to be 1-based)
pgsql 0-based
mysql 0-based (per MySQL docs for mysql_data_seek)
APR 1.3 drivers
oracle 1-based (per Oracle docs for "Scrollable Cursors in OCI" and OCIStmtFetch2)
freetds 1-based (per SQLServer docs for dbgetrow function)
I think this is a valid bug. Personally I would favor 1-based. The majority of dbd drivers already
do this, and most users would expect the traditional SQL meaning of "row number", i.e. 1-based row
numbers.
-tom-
Re: sqlite3 and postgresql
Posted by Bob Rossi <bo...@cox.net>.
On Mon, May 19, 2008 at 08:58:32AM +1000, Bojan Smojver wrote:
> On Sat, 2008-05-17 at 09:07 -0400, Bob Rossi wrote:
>
> > I've been using sqlite3 dbd for some time now. I find that the function
> > apr_dbd_get_row,
> > http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
> > is 1 based. That is, 1 is the first row returned from a query. However,
> > I just started using postgresql, and I find that apr_dbd_get_row is 0
> > based. That is, 0 is the first row returned from the query.
>
> This should now be fixed in both the trunk and 1.3.x branch.
>
> We really _do_ appreciate your testing and suggestions!
Sure, I'll test this. I'd hope that you could provide a solution to my
recently posted question. If so, I'll test them both in one shot!
Thanks!
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
On Wed, 2008-05-21 at 16:29 -0400, Bob Rossi wrote:
> Ok, all my unit tests pass on linux and windows. Thanks a million!
You are welcome. Thank you for testing!
--
Bojan
Re: sqlite3 and postgresql
Posted by Bob Rossi <bo...@cox.net>.
On Thu, May 22, 2008 at 05:43:27AM +1000, Bojan Smojver wrote:
> On Wed, 2008-05-21 at 10:13 -0400, Bob Rossi wrote:
>
> > OK, I have confirmed 1 bug. If you pass '0' to apr_dbd_get_row, with
> > postgresql as the driver, the function returns 0, instead of -1 as it
> > should. My unit tests happen to test the cases that should fail, that's
> > how I caught this.
>
> Try now. I also fixed MySQL driver to do the same.
Ok, all my unit tests pass on linux and windows. Thanks a million!
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
On Wed, 2008-05-21 at 10:13 -0400, Bob Rossi wrote:
> OK, I have confirmed 1 bug. If you pass '0' to apr_dbd_get_row, with
> postgresql as the driver, the function returns 0, instead of -1 as it
> should. My unit tests happen to test the cases that should fail, that's
> how I caught this.
Try now. I also fixed MySQL driver to do the same.
--
Bojan
Re: sqlite3 and postgresql
Posted by Bob Rossi <bo...@cox.net>.
On Wed, May 21, 2008 at 10:13:41AM -0400, Bob Rossi wrote:
> On Mon, May 19, 2008 at 09:32:07PM -0400, Bob Rossi wrote:
> > On Mon, May 19, 2008 at 08:58:32AM +1000, Bojan Smojver wrote:
> > > On Sat, 2008-05-17 at 09:07 -0400, Bob Rossi wrote:
> > >
> > > > I've been using sqlite3 dbd for some time now. I find that the function
> > > > apr_dbd_get_row,
> > > > http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
> > > > is 1 based. That is, 1 is the first row returned from a query. However,
> > > > I just started using postgresql, and I find that apr_dbd_get_row is 0
> > > > based. That is, 0 is the first row returned from the query.
> > >
> > > This should now be fixed in both the trunk and 1.3.x branch.
> > >
> > > We really _do_ appreciate your testing and suggestions!
> >
> > I can confirm that the latest 1.3.x snapshot on linux, for both sqlite3
> > and postgresql use a 1 based row index. I removed my hack, and things
> > work great. I'm going to test windows now, could take some time.
> > (perhaps get results tomorrow).
>
> OK, I have confirmed 1 bug. If you pass '0' to apr_dbd_get_row, with
> postgresql as the driver, the function returns 0, instead of -1 as it
> should. My unit tests happen to test the cases that should fail, that's
> how I caught this.
With a little more research, I noticed that this is simply an if on an
uninitialized variable.
230| if (row == NULL) {
231| row = apr_palloc(pool, sizeof(apr_dbd_row_t));
232| *rowp = row;
233| row->res = res;
234| if ( sequential ) {
235| row->n = 0;
236| }
237| else {
238| if (rownum > 0) {
239| row->n = --rownum;
240| }
241| }
242| }
243| else {
244| if ( sequential ) {
245| ++row->n;
246| }
247| else {
248| if (rownum > 0) {
249| row->n = --rownum;
250| }
251| }
252| }
253|
254+---> if (res->random) {
255| if (row->n >= res->ntuples) {
256| *rowp = NULL;
257| apr_pool_cleanup_run(pool, res->res, clear_result);
258| res->res = NULL;
259| return -1;
260| }
261| }
The execution is, 230, 231, 232, 233, 234, 238, 254, 255. On line, 255
row->n is checked, however, it was never initialized.
Hope this helps.
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bob Rossi <bo...@cox.net>.
On Mon, May 19, 2008 at 09:32:07PM -0400, Bob Rossi wrote:
> On Mon, May 19, 2008 at 08:58:32AM +1000, Bojan Smojver wrote:
> > On Sat, 2008-05-17 at 09:07 -0400, Bob Rossi wrote:
> >
> > > I've been using sqlite3 dbd for some time now. I find that the function
> > > apr_dbd_get_row,
> > > http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
> > > is 1 based. That is, 1 is the first row returned from a query. However,
> > > I just started using postgresql, and I find that apr_dbd_get_row is 0
> > > based. That is, 0 is the first row returned from the query.
> >
> > This should now be fixed in both the trunk and 1.3.x branch.
> >
> > We really _do_ appreciate your testing and suggestions!
>
> I can confirm that the latest 1.3.x snapshot on linux, for both sqlite3
> and postgresql use a 1 based row index. I removed my hack, and things
> work great. I'm going to test windows now, could take some time.
> (perhaps get results tomorrow).
OK, I have confirmed 1 bug. If you pass '0' to apr_dbd_get_row, with
postgresql as the driver, the function returns 0, instead of -1 as it
should. My unit tests happen to test the cases that should fail, that's
how I caught this.
When, I do the apr_dbd_select, I pass -1 to the random variable. The
documentation says,
- 1 to support random access to results (seek any row); 0 to support
only looping through results in order (async access - faster)
Now, when I do the apr_dbd_get_row, I pass in 0 as the row number.
The code in apr_dbd_pgsql.c says,
228: int sequential = ((rownum >= 0) && res->random) ? 0 : 1;
This returns sequential as 0, which I believe is correct. Now Here:
230: if (row == NULL) {
231: row = apr_palloc(pool, sizeof(apr_dbd_row_t));
232: *rowp = row;
233: row->res = res;
234: if ( sequential ) {
235: row->n = 0;
236: }
237: else {
238: if (rownum > 0) {
239: row->n = --rownum;
240: }
241: }
242: }
my row is NULL, and I eventually get to line 238, and not to
239, since rownum is 0. Finally, here:
254: if (res->random) {
255: if (row->n >= res->ntuples) {
256: *rowp = NULL;
257: apr_pool_cleanup_run(pool, res->res, clear_result);
258: res->res = NULL;
259: return -1;
260: }
261: }
res->random is true, so I get to line 255. However,
then row->n >= res->ntuples is false, so I skip the rest of the if
statement. At this point, I immediately drop to the end of the
function, which returns 0.
Is this enough information for you to determine where the bug is?
Thanks,
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bob Rossi <bo...@cox.net>.
On Mon, May 19, 2008 at 08:58:32AM +1000, Bojan Smojver wrote:
> On Sat, 2008-05-17 at 09:07 -0400, Bob Rossi wrote:
>
> > I've been using sqlite3 dbd for some time now. I find that the function
> > apr_dbd_get_row,
> > http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
> > is 1 based. That is, 1 is the first row returned from a query. However,
> > I just started using postgresql, and I find that apr_dbd_get_row is 0
> > based. That is, 0 is the first row returned from the query.
>
> This should now be fixed in both the trunk and 1.3.x branch.
>
> We really _do_ appreciate your testing and suggestions!
I can confirm that the latest 1.3.x snapshot on linux, for both sqlite3
and postgresql use a 1 based row index. I removed my hack, and things
work great. I'm going to test windows now, could take some time.
(perhaps get results tomorrow).
Bob Rossi
Re: sqlite3 and postgresql
Posted by Bojan Smojver <bo...@rexursive.com>.
On Sat, 2008-05-17 at 09:07 -0400, Bob Rossi wrote:
> I've been using sqlite3 dbd for some time now. I find that the function
> apr_dbd_get_row,
> http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1
> is 1 based. That is, 1 is the first row returned from a query. However,
> I just started using postgresql, and I find that apr_dbd_get_row is 0
> based. That is, 0 is the first row returned from the query.
This should now be fixed in both the trunk and 1.3.x branch.
We really _do_ appreciate your testing and suggestions!
--
Bojan