Horton

Thread

Commits

  1. Fix plan created for inherited UPDATE/DELETE with all tables excluded.

  1. 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Petr Fedorov <petr.fedorov@phystech.edu> — 2019-02-01T14:32:43Z

    Hello,

The following code snippet demonstrates the problem: the first select
passes and the second [select * from testf(FALSE)] fails. I would expect
that select * from testf(...); works without errors in both cases.

begin;

create table test (id integer, data char(1)) partition by list (id)
tablespace pg_default;
create table test_1 partition of test for values in (1) partition by
list (data);
create table test_1_a partition of test_1 for values in ('a');
create function testf(p boolean) returns setof test language 'plpgsql'
as $body$ begin return query update test set id=id where p returning *;
end; $body$;
insert into test (id, data) values (1, 'a');
select * from testf(TRUE);
select * from testf(FALSE);

rollback;   

The result:

ERROR: structure of query does not match function result type

SQL state: 42804

Detail: Number of returned columns (0) does not match expected column
count (2).

Context: PL/pgSQL function testf(boolean) line 1 at RETURN QUERY


I'm on  PostgreSQL 11.1 on x86_64-pc-linux-gnu, compiled by gcc (GCC)
4.8.5 20150623 (Red Hat 4.8.5-28), 64-bit on Centos 7

uname -a gives   Linux 3.10.0-862.11.6.el7.x86_64 #1 SMP Tue Aug 14
21:49:04 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

  2. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-06T07:35:09Z

    Hi,

On 2019/02/01 23:32, Petr Fedorov wrote:
> Hello,
> 
> The following code snippet demonstrates the problem: the first select
> passes and the second [select * from testf(FALSE)] fails. I would expect
> that select * from testf(...); works without errors in both cases.
> 
> begin;
> 
> create table test (id integer, data char(1)) partition by list (id)
> tablespace pg_default;
> create table test_1 partition of test for values in (1) partition by
> list (data);
> create table test_1_a partition of test_1 for values in ('a');
> create function testf(p boolean) returns setof test language 'plpgsql'
> as $body$ begin return query update test set id=id where p returning *;
> end; $body$;
> insert into test (id, data) values (1, 'a');
> select * from testf(TRUE);
> select * from testf(FALSE);
> 
> rollback;   
> 
> The result:
> 
> ERROR: structure of query does not match function result type
> 
> SQL state: 42804
> 
> Detail: Number of returned columns (0) does not match expected column
> count (2).
> 
> Context: PL/pgSQL function testf(boolean) line 1 at RETURN QUERY

Thanks for the report.  There indeed appears to be a bug here.

The problem seems to be with how planner handles an empty plan (due to
constant-FALSE qual) when the target table is an inheritance tree.  OP's
example contains a partitioned table, but I could reproduce it with
regular inheritance:

create table parent (id int);
create table child () inherits (parent);
create or replace function testf(p boolean) returns setof parent
  language 'plpgsql' as $body$
    begin
      return query update parent set id = id where p returning *;
    end;
$body$;

select * from testf(true);
 id
────
(0 rows)

select * from testf(false);
ERROR:  structure of query does not match function result type
DETAIL:  Number of returned columns (0) does not match expected column
count (1).
CONTEXT:  PL/pgSQL function testf(boolean) line 1 at RETURN QUERY

No problem when there is no inheritance:

drop function testf;
create table foo (like parent);
create or replace function testf(p boolean) returns setof foo
  language 'plpgsql' as $body$
    begin
      return query update foo set id = id where p returning *;
    end;
$body$;
select * from testf(false);
 id
────
(0 rows)


Mismatch between the query result type and the function result type occurs
in the inheritance case, because the targetlist of the plan for the UPDATE
query in testf's body is empty, whereas the function execution code
(pl_exec.c) expects it match the function's result type (set of parent).
It's empty because inheritance_planner sets an empty Result path when it
finds that all the children are excluded, but hasn't generated enough
state in the path's RelOptInfo and PlannerInfo such that the correct
targetlist could be set in the empty Result plan that's eventually created.

Attached patch seems to fix it.  It also adds a test in inherit.sql.

Thoughts?

Thanks,
Amit

  3. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-13T03:58:35Z

    On 2019/02/06 16:35, Amit Langote wrote:
> Hi,
> 
> On 2019/02/01 23:32, Petr Fedorov wrote:
>> Hello,
>>
>> The following code snippet demonstrates the problem: the first select
>> passes and the second [select * from testf(FALSE)] fails. I would expect
>> that select * from testf(...); works without errors in both cases.
>>
>> begin;
>>
>> create table test (id integer, data char(1)) partition by list (id)
>> tablespace pg_default;
>> create table test_1 partition of test for values in (1) partition by
>> list (data);
>> create table test_1_a partition of test_1 for values in ('a');
>> create function testf(p boolean) returns setof test language 'plpgsql'
>> as $body$ begin return query update test set id=id where p returning *;
>> end; $body$;
>> insert into test (id, data) values (1, 'a');
>> select * from testf(TRUE);
>> select * from testf(FALSE);
>>
>> rollback;   
>>
>> The result:
>>
>> ERROR: structure of query does not match function result type
>>
>> SQL state: 42804
>>
>> Detail: Number of returned columns (0) does not match expected column
>> count (2).
>>
>> Context: PL/pgSQL function testf(boolean) line 1 at RETURN QUERY
> 
> Thanks for the report.  There indeed appears to be a bug here.
> 
> The problem seems to be with how planner handles an empty plan (due to
> constant-FALSE qual) when the target table is an inheritance tree.  OP's
> example contains a partitioned table, but I could reproduce it with
> regular inheritance:
> 
> create table parent (id int);
> create table child () inherits (parent);
> create or replace function testf(p boolean) returns setof parent
>   language 'plpgsql' as $body$
>     begin
>       return query update parent set id = id where p returning *;
>     end;
> $body$;
> 
> select * from testf(true);
>  id
> ────
> (0 rows)
> 
> select * from testf(false);
> ERROR:  structure of query does not match function result type
> DETAIL:  Number of returned columns (0) does not match expected column
> count (1).
> CONTEXT:  PL/pgSQL function testf(boolean) line 1 at RETURN QUERY
> 
> No problem when there is no inheritance:
> 
> drop function testf;
> create table foo (like parent);
> create or replace function testf(p boolean) returns setof foo
>   language 'plpgsql' as $body$
>     begin
>       return query update foo set id = id where p returning *;
>     end;
> $body$;
> select * from testf(false);
>  id
> ────
> (0 rows)
> 
> 
> Mismatch between the query result type and the function result type occurs
> in the inheritance case, because the targetlist of the plan for the UPDATE
> query in testf's body is empty, whereas the function execution code
> (pl_exec.c) expects it match the function's result type (set of parent).
> It's empty because inheritance_planner sets an empty Result path when it
> finds that all the children are excluded, but hasn't generated enough
> state in the path's RelOptInfo and PlannerInfo such that the correct
> targetlist could be set in the empty Result plan that's eventually created.
> 
> Attached patch seems to fix it.  It also adds a test in inherit.sql.
> 
> Thoughts?

Will add this to next CF.

Thanks,
Amit

  4. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-02-21T22:18:10Z

    Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> On 2019/02/01 23:32, Petr Fedorov wrote:
>> ERROR: structure of query does not match function result type

> Thanks for the report.  There indeed appears to be a bug here.

Yup, for sure.  You don't actually need a function at all to see
that there's a problem: if you just execute
	UPDATE ... WHERE false RETURNING some-columns;
you will notice that the emitted resultset has zero columns.

> Attached patch seems to fix it.  It also adds a test in inherit.sql.

This isn't quite right, because what we actually need to return is the
RETURNING column set.  If you only check "RETURNING *" then you might not
notice the difference, but with anything else it's obviously wrong.
I propose the attached modification instead.

			regards, tom lane

  5. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-22T02:16:40Z

    On 2019/02/22 7:18, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> On 2019/02/01 23:32, Petr Fedorov wrote:
>>> ERROR: structure of query does not match function result type
> 
>> Thanks for the report.  There indeed appears to be a bug here.
> 
> Yup, for sure.  You don't actually need a function at all to see
> that there's a problem: if you just execute
> 	UPDATE ... WHERE false RETURNING some-columns;
> you will notice that the emitted resultset has zero columns.

Ah, indeed.

>> Attached patch seems to fix it.  It also adds a test in inherit.sql.
> 
> This isn't quite right, because what we actually need to return is the
> RETURNING column set.  If you only check "RETURNING *" then you might not
> notice the difference, but with anything else it's obviously wrong.
>
> I propose the attached modification instead.

Looks good.

I know this code may not be with us forever, but I wonder why the plan
shape looks different for an empty update on a regular table vs
inheritance tree.  For regular table, it's ModifyTable on top of a dummy
Result node, whereas it's just dummy Result node for the latter.  If the
plan shape for the two cases had matched, we wouldn't have this bug at all
or we'd have it for both cases (in slightly different form for the regular
table case).  To check I patched grouping_planner() to not add a
ModifyTable on top of a dummy result path for a regular table and the
resulting target list is not quite right (what you get with my patch
upthread):

create table foo (a int);
update foo set a = a where false returning a+1 as b;
 a
───
(0 rows)

explain verbose update foo set a = a where false returning a+1 as b;
                QUERY PLAN
───────────────────────────────────────────
 Result  (cost=0.00..0.00 rows=0 width=10)
   Output: a, ctid
   One-Time Filter: false
(3 rows)

Also, I noticed regression test failure having to do with statement
triggers not firing, which makes sense, as there's no ModifyTable to
invoke them.

That means we have a bug (?) today that statement triggers of inheritance
parent tables don't fire when it's an empty update/delete.

create table parent (a int, b int);
create table child () inherits (parent);
create or replace function before_stmt_notice() returns trigger as $$
begin raise notice 'updating %', TG_TABLE_NAME; return null; end; $$
language plpgsql;
create trigger before_stmt_trigger before update on parent execute
function before_stmt_notice();

-- trigger doesn't fire
update parent set a = a where false returning a+1 as b;
──
(0 rows)

It does fire for an empty update on a regular table (with HEAD I mean)

create trigger before_stmt_trigger before update on foo execute function
before_stmt_notice();
update foo set a = a where false returning a+1 as b;
NOTICE:  updating foo
 b
───
(0 rows)


Thanks,
Amit

  6. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-02-22T04:43:13Z

    Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> Also, I noticed regression test failure having to do with statement
> triggers not firing, which makes sense, as there's no ModifyTable to
> invoke them.

Oh!  You are right, that's a separate bug.  So really we can't have
this fast-path exit at all, we should produce a ModifyTable node in
every case.

I'm too tired to work on that anymore today, do you want to run
with it?

			regards, tom lane

  7. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2019-02-22T05:33:55Z

    On 2019/02/22 13:43, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> Also, I noticed regression test failure having to do with statement
>> triggers not firing, which makes sense, as there's no ModifyTable to
>> invoke them.
> 
> Oh!  You are right, that's a separate bug.  So really we can't have
> this fast-path exit at all, we should produce a ModifyTable node in
> every case.
> 
> I'm too tired to work on that anymore today, do you want to run
> with it?

Sure, see attached a patch.

To fix the trigger bug, we'll be putting a minimally valid-looking
ModifyTable node on top of a dummy plan.  So, the bug reported on this
thread is taken care of automatically, because ModifyTable plan's
targetlist is already set correctly.

Thanks,
Amit

  8. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-02-22T17:25:58Z

    Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> On 2019/02/22 13:43, Tom Lane wrote:
>> I'm too tired to work on that anymore today, do you want to run
>> with it?

> Sure, see attached a patch.

OK, I prettified this a bit and pushed it.

			regards, tom lane

  9. Re: 'update returning *' returns 0 columns instead of empty row with 2 columns when (i) no rows updated and (ii) when applied to a partitioned table with sub-partition

    Amit Langote <amitlangote09@gmail.com> — 2019-02-22T17:55:32Z

    On Sat, Feb 23, 2019 at 2:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
> > On 2019/02/22 13:43, Tom Lane wrote:
> >> I'm too tired to work on that anymore today, do you want to run
> >> with it?
>
> > Sure, see attached a patch.
>
> OK, I prettified this a bit and pushed it.

Thank you.

Regards,
Amit