Re: [Libdbi-drivers-devel] Wrong seeking in dbd_mysql

[Markus H. <mar...@mh...>]

Am 2014-05-01 18:50, schrieb Jan Engelhardt:
> I have identified another bug and prepared a patch for
> dbd_mysql/dbd_msql. The purpose of result->currowidx was not totally
> clear-cut, so I am posting this patch for potential comments.
> 
> 
> parent 1619049a1e0eb018cdc8490f3cf344b4f2f53b0f ()
> commit 537c2673f36465c2700829130da4054f53354a00
> Author: Jan Engelhardt <je...@in...>
> Date:   Thu May 1 18:04:47 2014 +0200
> 
> dbd_mysql, dbd_msql: resolve bogus seeking and a crash
> 
> I have a query result that, when iteratively seeked with
> dbi_result_next_row() and read with dbi_result_get_string(), yields
> these 15 values (1-based row indices shown in parentheses):
> 
> 	B-W(1) BAY(2) BER(3) BRE(4) HAM(5) HES(6) M-V(7) NDS(8)
> 	NRW(9) RLP(10) SAR(11) SAC(12) S-A(13) S-H(14) THU(15)
> 
> Now, when using dbi_result_seek_row() to seek to arbitrary rows
> (given in parentheses), dbi_result_get_string() can return incorrect
> values. In particular, I have observed:
> 
> 	B-W(1) S-H(14) B-W(1) THU(2)
> 
> What happens:
> 
> * row 1 is not already present, dbd_goto_row(1) is executed,
>   the row data retrieved, and result->currowidx set to 1
> * row 14 is not already present, a goto_row(14) is executed,
>   the row data retrieved, and result->currowidx set to 14
> * row 1 is already present, result->currowidx set to 1
> * row 2 is not already present, dbd_goto_row(2) is NOT executed,
>   because of rowidx == currowidx + 1 [2 == 2].
>   So the next mysql row fetched is row 15, rather than 2.
> 
> Now, if my seek order was {1, 15, 1, 2}, then mysql_fetch_row()
> called from function dbd_mysql.c:_get_row_data() will return NULL
> because there are no more rows. There will be a subsequent NULL
> dereference and crash.
> 
> It seems that result->currowidx is a high-level pointer, and is not
> synchronized to the current DBD row pointer.
> 
> As such, these checks should be killed and a goto_row always
> performed when so requested; the other DBD backends do the same.
> 
> After applying this patch, the seeked results correctly yield:
> 
> 	B-W(1) S-H(14) B-W(1) BAY(2)

Hi,

I can't offer a better solution off the top of my head, but you 
essentially revert a patch which was designed to avoid an expensive call 
to mysql_data_seek() in *every single* iteration of stepping through a 
result set. There are data in the mailing list archives which show that 
MySQL is slowed down abysmally in large result sets when accessed 
through libdbi in comparison to libmysqlclient. It would be great if we 
could find a way to synchronize result->currowidx with the DBD row 
pointer in a cheaper way.

regards,
Markus

-- 
Markus Hoenicka
http://www.mhoenicka.de
AQ score 38