(2014/08/12 18:34), Shigeru Hanada wrote:
> Issues addressed by Eitoku-san were fixed properly, but he found a bug
> and a possible enhancement in the v2 patch.
Thank you for the review, Hanada-san and Eitoku-san!
> * push-down check misses delete triggers
> update_is_pushdown_safe() seems to have a bug that it misses the
> existence of row-level delete trigger. DELETE statement executed
> against a foreign table which has row-level delete trigger is pushed
> down to remote, and consequently no row-level delete trigger is fired.
Ah, I noticed that the current code for that is not correct. Will fix.
> * further optimization
> Is there any chance to consider further optimization by passing the
> operation type (UPDATE|DELETE) of undergoing statement to
> update_is_pushdown_safe()? It seems safe to push down UPDATE
> statement when the target foreign table has no update trigger even it
> has a delete trigger (of course the opposite combination would be also
> fine).
Good idea! Will improve that too.
> * Documentation
> The requirement of pushing down UPDATE/DELETE statements would not be
> easy to understand for non-expert users, so it seems that there is a
> room to enhance documentation. An idea is to define which expression
> is safe to send to remote first (it might need to mention the
> difference of semantics), and refer the definition from the place
> describing the requirement of pushing-down for SELECT, UPDATE and
> DELETE.
Yeah, I also think that it would not necessarily easy for the users to
understand which expression is safe to send. So I agree with that
enhancement, but ISTM that it would be better to do that as a separate
patch.
Thanks,
Best regards,
Etsuro Fujita
