On 2016/10/07 10:26, Amit Langote wrote:
On 2016/10/06 21:55, Etsuro Fujita wrote:
On 2016/10/06 20:17, Amit Langote wrote:
On 2016/10/05 20:45, Etsuro Fujita wrote:

I noticed that we were wrong.  Your patch was modified so that
dependencies on FDW-related objects would be extracted from a given plan
in create_foreignscan_plan (by Ashutosh) and then in
set_foreignscan_references by me, but that wouldn't work well for INSERT
cases.  To fix that, I'd like to propose that we collect the dependencies
from the given rte in add_rte_to_flat_rtable, instead.

I see.  So, doing it from set_foreignscan_references() fails to capture
plan dependencies in case of INSERT because it won't be invoked at all
unlike the UPDATE/DELETE case.

Right.

If some writable FDW consulted foreign table/server/FDW options in
AddForeignUpdateTarget, which adds the extra junk columns for
UPDATE/DELETE to the targetList of the given query tree, the rewritten
query tree would also need to be invalidated.  But I don't think such an
FDW really exists because that routine in a practical FDW wouldn't change
such columns depending on those options.

I had second thoughts about that; since the possibility wouldn't be zero,
I added to extract_query_dependencies_walker the same code I added to
add_rte_to_flat_rtable.

And here, since AddForeignUpdateTargets() could possibly utilize foreign
options which would cause *query tree* dependencies.  It's possible that
add_rte_to_flat_rtable may not be called before an option change, causing
invalidation of any cached objects created based on the changed options.
So, must record dependencies from extract_query_dependencies as well.

Right.

I think this (v4) patch is in the best shape so far.

Thanks for the review!

Best regards,
Etsuro Fujita




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to