Re: inherit support for foreign tables

Hello,

At Thu, 16 Apr 2015 12:20:47 +0900, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <552F2A8F(dot)2090406(at)lab(dot)ntt(dot)co(dot)jp>
> On 2015/04/15 3:52, Alvaro Herrera wrote:
> >> On 4/14/15 5:49 AM, Etsuro Fujita wrote:
> >>> postgres=# create foreign table ft1 (c1 int) server myserver options
> >>> (table_name 't1');
> >>> CREATE FOREIGN TABLE
> >>> postgres=# create foreign table ft2 (c1 int) server myserver options
> >>> (table_name 't2');
> >>> CREATE FOREIGN TABLE
> >>> postgres=# alter foreign table ft2 inherit ft1;
> >>> ALTER FOREIGN TABLE
> >>> postgres=# select * from ft1 for update;
> >>> ERROR: could not find junk tableoid1 column
> >>>
> >>> I think this is a bug. Attached is a patch fixing this issue.
>
> > I think the real question is whether we're now (I mean after this
> > patch)
> > emitting useless tableoid columns that we didn't previously have. I
> > think the answer is yes, and if so I think we need a smaller comb to
> > fix
> > the problem. This one seems rather large.
>
> My answer for that would be *no* because I think tableoid would be
> needed for EvalPlanQual checking in more complex SELECT FOR UPDATE on
> the inheritance or UPDATE/DELETE involving the inheritance as a source
> table. Also, if we allow the FDW to change the behavior of SELECT FOR
> UPDATE so as to match the local semantics exactly, which I'm working
> on in another thread, I think tableoid would also be needed for the
> actual row locking.

Given the parent foreign talbes, surely they need tableoids for
such usage. The patch preserves the condition rc->isParent so it
newly affects exactly only parent foreign tables for now.

Before the parent foreign tables introduced, ROW_MARK_COPY and
RTE_RELATION are mutually exclusive so didn't need, or cannot
have tableoid. But now it intorduces an rte with ROW_MARK_COPY &
RTE_RELATION and there seems no reason for parent tables in any
kind not to have tableoid. After such consideration, I came to
think that the patch is a reasonable fix, not mere a workaround.

Thoughts?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center