On 10/24/14, 6:17 PM, Jim Nasby wrote:
>>>> - Does anyone have a tangible suggestion for how to reduce the code
>>>> duplication in patch #6?
>>>
>>> Between execute_sql_string() and tcop/exec_simple_query()? Is there stuff in
>>> exec_simple that's not safe for bgwriter? I'm not seeing why we can't use
>>> exec_simple. :/
>>
>> There are some differences if you compare them closely.
>
> Without doing a deep dive, I'm guessing that most of the extra stuff would be safe to re-use; it just wouldn't affect
execute_sql_string.Obviously we could add a boolean to exec_simple_query for the case when it's being used by a
bgwriter.Though, it seems like the biggest differences have to do with logging
>
> Here's the differences I see:
>
> - Disallowing transaction commands
> - Logging
> - What memory context is used (could we just use that differently in a pg_backend backend?)
> - Portal output format
> - What to do with the output of intermediate commands (surely there's other places we need to handle that? plpgsql
maybe?)
>
> I think all of those except logging could be handled by a function serving both exec_simple_query and
execute_sql_commandthat accepts a few booleans (or maybe just one to indicate the caller) and some if's. At least I
don'tthink it'd be too bad, without actually writing it.
>
> I'm not sure what to do about logging. If the original backend has logging turned on, is it that horrible to do the
samelogging as exec_simple_query would do?
>
> Another option would be factoring out parts of exec_simple_query; the for loop over the parse tree might be a good
candidate.But I suspect that would be uglier than a separate support function.
>
> I do feel uncomfortable with the amount of duplication there is right now though...
I took a stab at this by refactoring the guts of exec_simple_query (patch attached) into a new function called
exec_query_string(also attached in raw form). As indicated it needs a bit more work. In particular, how it's dealing
withexcluding transactional commands is rather ugly. Why do we need to do that in pg_background?
Andres was concerned about the performance impact of doing this. I tested this by doing
for i in {1..999999}; do echo 'SELECT 1;' >> test.sql; done
and then
time psql -f test.sql > /dev/nul
It appears there may be a ~1% performance hit, but my laptop isn't repeatable enough to be sure. I did try manually
in-liningexec_query_string to see if it was the function call causing the issue; it didn't seem to make a difference.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
