Обсуждение: Re: [BUGS] BUG #4599: bugfix for contrib/dblink module
Oleksiy Shchukin wrote: > Reproduce how-to > ---------------- > I send bad SQL statement via dblink_send_query(text,text), and than try to > grab result with dblink_get_result(connname, /*fail_on_error=*/ false), the > bad sql statement error is issued on client side inspite of > '/*fail_on_error=*/ false' parameter is passed to dblink_get_result() On line 795 below, fail should get set to PG_GETARG_BOOL(1). However, as line 842 is about to be executed, fail is still set to true, even though PG_GETARG_BOOL(1) is clearly false. Any ideas? 8<----------------------------------------------------------- 788 else if (is_async && do_get) (gdb) 791 if (PG_NARGS() == 2) (gdb) 794 DBLINK_GET_CONN; (gdb) 795 fail = PG_GETARG_BOOL(1); (gdb) 819 if (!conn) (gdb) 822 if (!is_async || (is_async && do_get)) (gdb) 825 if (!is_async) (gdb) 829 res = PQgetResult(conn); (gdb) 831 if (!res) (gdb) 838 if (!res || (gdb) 842 dblink_res_error(conname, res, "could not execute query", fail); (gdb) p fail $8 = 1 '\001' (gdb) print PG_GETARG_BOOL(1) $9 = 0 '\0' 8<----------------------------------------------------------- Joe
Joe Conway <mail@joeconway.com> writes: > On line 795 below, fail should get set to PG_GETARG_BOOL(1). However, as > line 842 is about to be executed, fail is still set to true, even though > PG_GETARG_BOOL(1) is clearly false. Any ideas? I can't duplicate that here, but my first reaction on studying this code is "ick!". Having a non-set-returning function calling the SRF infrastructure (and not bothering to clean it up on exit, either) is just horrid --- I have no idea what side-effects that might have, but at the very least there's going to be a memory leak. Trying to implement three significantly different functions as one function with a maze of if's is not good style in any case. I think you should break those three functions apart. There is no value in having send_query share any code with the others. It might be feasible to have the other two share a subroutine that collects the result data. regards, tom lane
Tom Lane wrote: > I think you should break those three functions apart. There is no value > in having send_query share any code with the others. It might be > feasible to have the other two share a subroutine that collects the > result data. OK, will do. I applied the simple fix to the 8.2 and 8.3 branches, but will do refactoring per your suggestion on cvs tip. Thanks, Joe