http://www.slony.info/bugzilla/show_bug.cgi?id=167

--- Comment #4 from Steve Singer <ssinger at ca.afilias.info> 2010-12-14 10:56:08 PST ---
(In reply to comment #1)
> I have combined the EXPLAIN_INTERVAL feature with the new log selection query
> into one commit for review here:
> 
> https://github.com/wieck/slony1-engine/commit/cfdf5c511e390775fdeb43dc37b07dffc61a3b20
> 

My initial review is here

remote_worker.c:4008
1) This switch statement , switch(provider->log_status) 
a) Inside of it we reach 5 levels of indentation. I think the code
will be easier to read/follow if we some of this into a separate
function

b) From what I can tell we have a switch(provier->log_status) with
the same code for 3 case statements (0,2,3) and those are the only
case statements in the switch block that ends at line 4144.

There is then a second switch(provider->log_status) with three more
statements of the same form.

if(provider->log_status==0 || provider->log_status==2 ||
provider->log_status==3)
would be a lot easier to read/follow. This might even make the 
formatting easier to read to avoid (a)

However I thik you can go further and create a function that takes in it
an argument (1 or 2) along with a pointer to provider_query and
appends the parts to it using either sl_log_%d (sl_log_1 or sl_log_2)
as required.  There is a lot of code duplication here that I think can
be eliminated.

Once the duplication has been eliminated I'll take another look.

I agree with Chris that a plpgsql implementation might be worth trying.

-- 
Configure bugmail: http://www.slony.info/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.