Re: Update does not move row across foreign partitions in v11

On Wed, 6 Mar 2019 at 16:29, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
> (2019/03/06 11:06), David Rowley wrote:
> > I don't quite understand what a "foreign table to some other
> > partition" is meant to mean. Partitions don't have foreign tables,
> > they can only be one themselves.
>
> I think "foreign table" is describing "partition" in front of that; "a
> partition that is a foreign table".

I think I was reading this wrong:

- Currently, rows cannot be moved from a partition that is a
- foreign table to some other partition, but they can be moved into a foreign
- table if the foreign data wrapper supports it.

I parsed it as "cannot be moved from a partition, that is a foreign
table to some other partition"

and subsequently struggled with what "a foreign table to some other
partition" is.

but now looking at it, I think it's meant to mean:

"cannot be moved from a foreign table partition to another partition"

> > I've tried to put all this right again in the attached. However, I was
> > a bit unsure of what "but they can be moved into a foreign table if
> > the foreign data wrapper supports it." is referring to. Copying Robert
> > and Etsuro as this was all added in 3d956d9562aa. Hopefully, they can
> > confirm what is meant by this.
>
> That means that rows can be moved from a local partition to a foreign
> partition if the FDW supports it.

It seems a bit light on detail to me. If I was a user I'd want to know
what exactly the FDW needed to support this. Does it need a special
partition move function? Looking at ExecFindPartition(), this check
seems to be done in CheckValidResultRel() and is basically:

case RELKIND_FOREIGN_TABLE:
/* Okay only if the FDW supports it */
fdwroutine = resultRelInfo->ri_FdwRoutine;
switch (operation)
{
case CMD_INSERT:
if (fdwroutine->ExecForeignInsert == NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot insert into foreign table \"%s\"",
RelationGetRelationName(resultRel))));

Alternatively, we could just remove the mention about "if the FDW
supports it", since it's probably unlikely for an FDW not to support
INSERT.

> IMO, I think the existing mention in [3] is good, so I would vote for
> putting the same mention in table 5.10.2.3 in [2] as well.

I think the sentence is unclear, at least I struggled to parse it the
first time. Happy for Amit to choose some better words and include in
his patch. I think it should be done in the same commit.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services