Fujita-san, On Thu, Feb 22, 2018 at 8:49 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote: > (2018/02/22 11:52), Amit Langote wrote: >> I wonder why partition_index needs to be made part of this API? > > The reason for that is because I think the FDW might want to look at the > partition info stored in mtstate->mt_partition_tuple_routing for some reason > or other, such as parent_child_tupconv_maps and child_parent_tupconv_maps, > which are accessed with the given partition index.
I see. I guess that makes sense. >> Perhaps an independent concern, but one thing I noticed is that it does >> not seem to play well with the direct modification (update push-down) >> feature. Now because updates (at least local, let's say) support >> re-routing, I thought we'd be able move rows across servers via the local >> server, but with direct modification we'd never get the chance. However, >> since update tuple routing is triggered by partition constraint failure, >> which we don't enforce for foreign tables/partitions anyway, I'm not sure >> if we need to do anything about that, and even if we did, whether it >> concerns this proposal. > > Good point! Actually, update tuple routing we have in HEAD doesn't allow > re-routing tuples from foreign partitions even without direct modification. > Here is an example using postgres_fdw: > > postgres=# create table pt (a int, b text) partition by list (a); > CREATE TABLE > postgres=# create table loct (a int check (a in (1)), b text); > CREATE TABLE > postgres=# create foreign table remp partition of pt for values in (1) > server loopback options (table_name 'loct'); > CREATE FOREIGN TABLE > postgres=# create table locp partition of pt for values in (2); > CREATE TABLE > postgres=# insert into remp values (1, 'foo'); > INSERT 0 1 > postgres=# insert into locp values (2, 'bar'); > INSERT 0 1 > postgres=# select tableoid::regclass, * from pt; > tableoid | a | b > ----------+---+----- > remp | 1 | foo > locp | 2 | bar > (2 rows) > > postgres=# create function postgres_fdw_abs(int) returns int as $$begin > return abs($1); end$$ language plpgsql immutable; > CREATE FUNCTION > postgres=# explain verbose update pt set a = postgres_fdw_abs(a) + 1 where b > = 'foo'; > QUERY PLAN > > -------------------------------------------------------------------------------- > ------------- > Update on public.pt (cost=100.00..154.54 rows=12 width=42) > Foreign Update on public.remp > Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1 > Update on public.locp > -> Foreign Scan on public.remp (cost=100.00..127.15 rows=6 width=42) > Output: (postgres_fdw_abs(remp.a) + 1), remp.b, remp.ctid > Remote SQL: SELECT a, b, ctid FROM public.loct WHERE ((b = > 'foo'::text)) FOR UPDATE > -> Seq Scan on public.locp (cost=0.00..27.39 rows=6 width=42) > Output: (postgres_fdw_abs(locp.a) + 1), locp.b, locp.ctid > Filter: (locp.b = 'foo'::text) > (10 rows) > > postgres=# update pt set a = postgres_fdw_abs(a) + 1 where b = 'foo'; > ERROR: new row for relation "loct" violates check constraint "loct_a_check" > DETAIL: Failing row contains (2, foo). > CONTEXT: Remote SQL command: UPDATE public.loct SET a = $2 WHERE ctid = $1 > > To be able to move tuples across foreign servers, I think we would first > have to do something to allow re-routing tuples from foreign partitions. > The patches I proposed hasn't done anything about that, so the patched > version would behave the same way as HEAD with/without direct modification. Yes. As I said, since update re-routing is triggered by partition constraint failure for the new row and we don't check (any) constraints for foreign tables, that means re-routing won't occur for foreign partitions. >> That said, I saw in the changes to ExecSetupPartitionTupleRouting() that >> BeginForeignRouting() is called for a foreign partition even if direct >> modification might already have been set up. If direct modification is >> set up, then ExecForeignRouting() will never get called, because we'd >> never call ExecUpdate() or ExecInsert(). > > Yeah, but I am thinking to leave the support for re-routing tuples across > foreign servers for another patch. > > One difference between HEAD and the patched version would be: we can > re-route tuples from a plain partition to a foreign partition if the foreign > partition supports this tuple-routing API. Here is an example using the > above data: > > postgres=# select tableoid::regclass, * from pt; > tableoid | a | b > ----------+---+----- > remp | 1 | foo > locp | 2 | bar > (2 rows) > > postgres=# update pt set a = 1 where b = 'bar'; > UPDATE 1 > postgres=# select tableoid::regclass, * from pt; > tableoid | a | b > ----------+---+----- > remp | 1 | foo > remp | 1 | bar > (2 rows) > > This would introduce an asymmetry (we can move tuples from plain partitions > to foreign partitions, but the reverse is not true), but I am thinking that > it would be probably okay to document about that. I see. Thanks for the explanation. About just documenting the asymmetry you mentioned that's caused by the fact that we don't enforce constraints on foreign tables, I started wondering if we shouldn't change our stance on the matter wrt "partition" constraints? But, admittedly, that's a topic for a different thread. Thanks, Amit
