Horton

Thread

Commits

  1. When WCOs are present, disable direct foreign table modification.

  1. Mishandling of WCO constraints in direct foreign table modification

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2017-07-20T11:40:31Z

    Here is an example for $subject using postgres_fdw:

postgres=# create foreign table foreign_tbl (a int, b int) server 
loopback options (table_name 'base_tbl');
CREATE FOREIGN TABLE
postgres=# create view rw_view as select * from foreign_tbl where a < b 
with check option;
CREATE VIEW
postgres=# insert into rw_view values (0, 10);
INSERT 0 1
postgres=# explain verbose update rw_view set a = 20 where b = 10;
                                       QUERY PLAN
--------------------------------------------------------------------------------------
  Update on public.foreign_tbl  (cost=100.00..146.21 rows=4 width=14)
    ->  Foreign Update on public.foreign_tbl  (cost=100.00..146.21 
rows=4 width=14)
          Remote SQL: UPDATE public.base_tbl SET a = 20 WHERE ((a < b)) 
AND ((b = 10))
(3 rows)

postgres=# update rw_view set a = 20 where b = 10;
UPDATE 1

This is wrong!  This should fail.  The reason for that is; direct modify 
is overlooking checking WITH CHECK OPTION constraints from parent views. 
  I think we could do direct modify, even if there are any WITH CHECK 
OPTIONs, in some way or other, but I think that is a feature.  So, I'd 
like to propose to fix this by just giving up direct modify if there are 
any WITH CHECK OPTIONs.  Attached is a patch for that.  I'll add it to 
the next commitfest.

Best regards,
Etsuro Fujita

  2. Re: Mishandling of WCO constraints in direct foreign table modification

    Robert Haas <robertmhaas@gmail.com> — 2017-07-20T18:24:20Z

    On Thu, Jul 20, 2017 at 7:40 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> Here is an example for $subject using postgres_fdw:
>
> postgres=# create foreign table foreign_tbl (a int, b int) server loopback
> options (table_name 'base_tbl');
> CREATE FOREIGN TABLE
> postgres=# create view rw_view as select * from foreign_tbl where a < b with
> check option;
> CREATE VIEW
> postgres=# insert into rw_view values (0, 10);
> INSERT 0 1
> postgres=# explain verbose update rw_view set a = 20 where b = 10;
>                                       QUERY PLAN
> --------------------------------------------------------------------------------------
>  Update on public.foreign_tbl  (cost=100.00..146.21 rows=4 width=14)
>    ->  Foreign Update on public.foreign_tbl  (cost=100.00..146.21 rows=4
> width=14)
>          Remote SQL: UPDATE public.base_tbl SET a = 20 WHERE ((a < b)) AND
> ((b = 10))
> (3 rows)
>
> postgres=# update rw_view set a = 20 where b = 10;
> UPDATE 1
>
> This is wrong!  This should fail.  The reason for that is; direct modify is
> overlooking checking WITH CHECK OPTION constraints from parent views.  I
> think we could do direct modify, even if there are any WITH CHECK OPTIONs,
> in some way or other, but I think that is a feature.  So, I'd like to
> propose to fix this by just giving up direct modify if there are any WITH
> CHECK OPTIONs.  Attached is a patch for that.  I'll add it to the next
> commitfest.

I think that's reasonable.  This should be committed and back-patched
to 9.6, right?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  3. Re: Mishandling of WCO constraints in direct foreign table modification

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2017-07-21T03:00:03Z

    On 2017/07/21 3:24, Robert Haas wrote:
> I think that's reasonable.  This should be committed and back-patched
> to 9.6, right?

Yeah, because direct modify was introduced in 9.6.

Attached is the second version which updated docs in postgres-fdw.sgml 
as well.

Best regards,
Etsuro Fujita

  4. Re: Mishandling of WCO constraints in direct foreign table modification

    Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> — 2017-07-21T08:18:37Z

    At Fri, 21 Jul 2017 12:00:03 +0900, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote in <15aa9936-9bd8-c9e3-7ca1-3948610734b4@lab.ntt.co.jp>
> On 2017/07/21 3:24, Robert Haas wrote:
> > I think that's reasonable.  This should be committed and back-patched
> > to 9.6, right?
> 
> Yeah, because direct modify was introduced in 9.6.
> 
> Attached is the second version which updated docs in postgres-fdw.sgml
> as well.

!    no local joins for the query, no row-level local <literal>BEFORE</> or
!    <literal>AFTER</> triggers on the target table, and no
!    <literal>CHECK OPTION</> constraints from parent views.
!    In <command>UPDATE</>,

Might be a silly question, is CHECK OPTION a "constraint"?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  5. Re: Mishandling of WCO constraints in direct foreign table modification

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2017-07-21T10:21:42Z

    On 2017/07/21 17:18, Kyotaro HORIGUCHI wrote:
> At Fri, 21 Jul 2017 12:00:03 +0900, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote in <15aa9936-9bd8-c9e3-7ca1-3948610734b4@lab.ntt.co.jp>
>> Attached is the second version which updated docs in postgres-fdw.sgml
>> as well.
> 
> !    no local joins for the query, no row-level local <literal>BEFORE</> or
> !    <literal>AFTER</> triggers on the target table, and no
> !    <literal>CHECK OPTION</> constraints from parent views.
> !    In <command>UPDATE</>,
> 
> Might be a silly question, is CHECK OPTION a "constraint"?

I mean constraints derived from WITH CHECK OPTIONs specified for parent 
views.  We use the words "WITH CHECK OPTION constraints" in comments in 
nodeModifyTable.c, so the expression "CHECK OPTION constrains" doesn't 
sound not that bad to me.  (I used "CHECK OPTION", not "WITH CHECK 
OPTION", because we use "CHECK OPTION" a lot more in the documentation 
than "WITH CHECK OPTION".)

Best regards,
Etsuro Fujita

  6. Re: Mishandling of WCO constraints in direct foreign table modification

    Robert Haas <robertmhaas@gmail.com> — 2017-07-24T20:35:30Z

    On Fri, Jul 21, 2017 at 6:21 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> I mean constraints derived from WITH CHECK OPTIONs specified for parent
> views.  We use the words "WITH CHECK OPTION constraints" in comments in
> nodeModifyTable.c, so the expression "CHECK OPTION constrains" doesn't sound
> not that bad to me.  (I used "CHECK OPTION", not "WITH CHECK OPTION",
> because we use "CHECK OPTION" a lot more in the documentation than "WITH
> CHECK OPTION".)

Yeah, it seems OK to me, too; if the consensus is otherwise, we also
have the option to change it later.  Committed and back-patched as you
had it, but I removed a spurious comma.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  7. Re: Mishandling of WCO constraints in direct foreign table modification

    Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> — 2017-07-26T02:50:05Z

    On 2017/07/25 5:35, Robert Haas wrote:
> On Fri, Jul 21, 2017 at 6:21 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> I mean constraints derived from WITH CHECK OPTIONs specified for parent
>> views.  We use the words "WITH CHECK OPTION constraints" in comments in
>> nodeModifyTable.c, so the expression "CHECK OPTION constrains" doesn't sound
>> not that bad to me.  (I used "CHECK OPTION", not "WITH CHECK OPTION",
>> because we use "CHECK OPTION" a lot more in the documentation than "WITH
>> CHECK OPTION".)
> 
> Yeah, it seems OK to me, too; if the consensus is otherwise, we also
> have the option to change it later.
Agreed.

> Committed and back-patched as you
> had it, but I removed a spurious comma.

Thanks for that, Robert!  Thanks for reviewing, Horiguchi-san!

Best regards,
Etsuro Fujita