Paul J Stevens wrote:
Yep. Its in dbmysql.c all right.
I hateses globals :-/
Storing the last_row_nr in a global variable is a flawed approach. I
assume a static like last_row_nr is an optimization that wants to
outsmart the mysql api. Such optimizations however, should, have and
will occur in the mysql libs. So, is there a solid, benchmarked reason
for avoiding use of mysql_data_seek and mysql_fetch_row.
This was done in November, after Alex Wheeler reported that
mysql_data_seek() was way to slow. I agree that these optimizations
belong in the libs, not in the code using the libs.
From mailing.database.mysql, 2001-06-21:
""
mysql_store_result stores the result set in a linked list (for memory
efficiency). With older versions mysql_data_seek always starts from
the beginning of the list and therefore has an O(n) complexity even
for sequential traversal.
IIRC, this was fixed with newer MySQL client libs, you either want to
upgrade or write some small wrapper.
""
So, this supports your stance, more so because we demand MySQL version
>= 2.0.14 anyway. Those versions should have this optimized internally.
I guess Alex was using MySQL 3.xx. (I could be wrong of course)
My trivial patch:
diff -r1.84 dbmysql.c
156a157
> /*
163a165,167
> */
> mysql_data_seek(res, row);
> last_row = mysql_fetch_row(res);
fixes the problem I've been having with _ic_list.
OK, I'll fix this and commit it.
Looking into this some further I noticed db_query tries to avoid
mysql_store_result, where the mysql docs clearly state such calls are ok:
------ info: mysql_store_result -----
You must call `mysql_store_result()' or `mysql_use_result()' for every
query that successfully retrieves data (`SELECT', `SHOW', `DESCRIBE',
`EXPLAIN').
You don't have to call `mysql_store_result()' or `mysql_use_result()'
for other queries, but it will not do any harm or cause any notable
performance if you call `mysql_store_result()' in all cases. You can
detect if the query didn't have a result set by checking if
`mysql_store_result()' returns 0 (more about this later one).
------------------------------------
Reading this db_query just doesn't look right to me. What do you think
Ilja ?
I took this code from earlier DBMail versions without thinking about it
too much.
So, we could just do a mysql_store_result(), and res will be set to NULL
when appropriate (i.e. a after a query which returns no result)? Nice.
This will make the function a lot simpler.
I guess we should do this then.
Btw, the testsuite now runs ok with the patch mentioned :-) So can we
clean up dbmysql.c a little and move on ?
I guess so, I'll get to it.
I read about you fixing the sigpipe error. Have you committed any
changes to that effect?
Haven't committed anything yet. I was wondering about the effects of the
'return EOF' statements after a ci_write() fails, because nothing is
cleaned up before them. I guess we should first clean up before
returning, don't you think?
Ilja
