Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Getting ERROR with FOR UPDATE/SHARE for partitioned table.

rajkumar.raghuwanshi
Hi All,

I am getting ERROR when using the "FOR UPDATE" clause for the partitioned table. below is a reproducible test case for the same.

CREATE TABLE tbl (c1 INT,c2 TEXT) PARTITION BY LIST (c1);
CREATE TABLE tbl_null PARTITION OF tbl FOR VALUES IN (NULL);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES IN (1,2,3);

INSERT INTO tbl SELECT i,i FROM generate_series(1,3) i;

CREATE OR REPLACE FUNCTION func(i int) RETURNS int
AS $$
DECLARE
 v_var tbl%ROWTYPE;
 cur CURSOR IS SELECT * FROM tbl WHERE c1< 5 FOR UPDATE;
BEGIN
 OPEN cur;
 LOOP
  FETCH cur INTO v_var;
  EXIT WHEN NOT FOUND;
  UPDATE tbl SET c2='aa' WHERE CURRENT OF cur;
 END LOOP;
 CLOSE cur;
 RETURN 10;
END;
$$ LANGUAGE PLPGSQL;

SELECT func(10);

postgres=# SELECT func(10);
ERROR:  cursor "cur" does not have a FOR UPDATE/SHARE reference to table "tbl_null"
CONTEXT:  SQL statement "UPDATE tbl SET c2='aa' WHERE CURRENT OF cur"
PL/pgSQL function func(integer) line 10 at SQL statement

Thanks & Regards,
Rajkumar Raghuwanshi
Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

amul sul

On Fri, May 22, 2020 at 5:00 PM Rajkumar Raghuwanshi <[hidden email]> wrote:
Hi All,

I am getting ERROR when using the "FOR UPDATE" clause for the partitioned table. below is a reproducible test case for the same.

CREATE TABLE tbl (c1 INT,c2 TEXT) PARTITION BY LIST (c1);
CREATE TABLE tbl_null PARTITION OF tbl FOR VALUES IN (NULL);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES IN (1,2,3);

INSERT INTO tbl SELECT i,i FROM generate_series(1,3) i;

CREATE OR REPLACE FUNCTION func(i int) RETURNS int
AS $$
DECLARE
 v_var tbl%ROWTYPE;
 cur CURSOR IS SELECT * FROM tbl WHERE c1< 5 FOR UPDATE;
BEGIN
 OPEN cur;
 LOOP
  FETCH cur INTO v_var;
  EXIT WHEN NOT FOUND;
  UPDATE tbl SET c2='aa' WHERE CURRENT OF cur;
 END LOOP;
 CLOSE cur;
 RETURN 10;
END;
$$ LANGUAGE PLPGSQL;

SELECT func(10);
 
I tried similar things on inherit partitioning as follow and that looks fine:

DROP TABLE tbl;
CREATE TABLE tbl (c1 INT,c2 TEXT);
CREATE TABLE tbl_null(check (c1 is NULL)) INHERITS (tbl);
CREATE TABLE tbl_1 (check (c1 > 0 and c1 < 4)) INHERITS (tbl);
INSERT INTO tbl_1 VALUES(generate_series(1,3));

postgres=# SELECT func(10);
 func
------
   10
(1 row)

On looking further for declarative partition, I found that issue happens only if
the partitioning pruning enabled, see this:

-- Execute on original set of test case.
postgres=# ALTER FUNCTION func SET enable_partition_pruning to off;
ALTER FUNCTION

postgres=# SELECT func(10);
 func
------
   10
(1 row)

I think we need some indication in execCurrentOf() to skip error if the relation
is pruned.  Something like that we already doing for inheriting partitioning,
see following comment execCurrentOf():

        /*  
         * This table didn't produce the cursor's current row; some other
         * inheritance child of the same parent must have.  Signal caller to
         * do nothing on this table.
         */

Regards,
Amul 
Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Amit Langote
On Fri, May 22, 2020 at 9:09 PM amul sul <[hidden email]> wrote:

> I tried similar things on inherit partitioning as follow and that looks fine:
>
> DROP TABLE tbl;
> CREATE TABLE tbl (c1 INT,c2 TEXT);
> CREATE TABLE tbl_null(check (c1 is NULL)) INHERITS (tbl);
> CREATE TABLE tbl_1 (check (c1 > 0 and c1 < 4)) INHERITS (tbl);
> INSERT INTO tbl_1 VALUES(generate_series(1,3));
>
> postgres=# SELECT func(10);
>  func
> ------
>    10
> (1 row)
>
> On looking further for declarative partition, I found that issue happens only if
> the partitioning pruning enabled, see this:
>
> -- Execute on original set of test case.
> postgres=# ALTER FUNCTION func SET enable_partition_pruning to off;
> ALTER FUNCTION
>
> postgres=# SELECT func(10);
>  func
> ------
>    10
> (1 row)
>
> I think we need some indication in execCurrentOf() to skip error if the relation
> is pruned.  Something like that we already doing for inheriting partitioning,
> see following comment execCurrentOf():
>
>         /*
>          * This table didn't produce the cursor's current row; some other
>          * inheritance child of the same parent must have.  Signal caller to
>          * do nothing on this table.
>          */

Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
would fail even with traditional inheritance:

drop table if exists p cascade;
create table p (a int);
create table c (check (a = 2)) inherits (p);
insert into p values (1);
insert into c values (2);
begin;
declare c cursor for select * from p where a = 1;
fetch c;
update p set a = a where current of c;
ERROR:  cursor "c" is not a simply updatable scan of table "c"
ROLLBACK

When there are no RowMarks to use because no FOR SHARE/UPDATE clause
was specified when declaring the cursor, execCurrentOf() tries to find
the cursor's current table by looking up its Scan node in the plan
tree but will not find it if it was excluded in the cursor's query.

With FOR SHARE/UPDATE, it seems to work because the planner delivers
the RowMarks of all the children irrespective of whether or not they
are present in the plan tree itself (something I had complained about
in past [1]).  execCurrentOf() doesn't complain as long as there is a
RowMark present even if it's never used.  For partitioning, the
planner doesn't make RowMarks for pruned partitions, so
execCurrentOf() can't find one if it's passed a pruned partition's
oid.

I don't see a way to avoid these errors.  How does execCurrentOf()
distinguish a table that could *never* be present in a cursor from a
table that could be had it not been pruned/excluded?  If it can do
that, then give an error for the former and return false for the
latter.

I guess the workaround is to declare the cursor such that no
partitions/children are pruned/excluded.

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com

[1] https://www.postgresql.org/message-id/468c85d9-540e-66a2-1dde-fec2b741e688%40lab.ntt.co.jp


Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Ashutosh Bapat-2
On Wed, May 27, 2020 at 12:53 PM Amit Langote <[hidden email]> wrote:

>
> On Fri, May 22, 2020 at 9:09 PM amul sul <[hidden email]> wrote:
> > I tried similar things on inherit partitioning as follow and that looks fine:
> >
> > DROP TABLE tbl;
> > CREATE TABLE tbl (c1 INT,c2 TEXT);
> > CREATE TABLE tbl_null(check (c1 is NULL)) INHERITS (tbl);
> > CREATE TABLE tbl_1 (check (c1 > 0 and c1 < 4)) INHERITS (tbl);
> > INSERT INTO tbl_1 VALUES(generate_series(1,3));
> >
> > postgres=# SELECT func(10);
> >  func
> > ------
> >    10
> > (1 row)
> >
> > On looking further for declarative partition, I found that issue happens only if
> > the partitioning pruning enabled, see this:
> >
> > -- Execute on original set of test case.
> > postgres=# ALTER FUNCTION func SET enable_partition_pruning to off;
> > ALTER FUNCTION
> >
> > postgres=# SELECT func(10);
> >  func
> > ------
> >    10
> > (1 row)
> >
> > I think we need some indication in execCurrentOf() to skip error if the relation
> > is pruned.  Something like that we already doing for inheriting partitioning,
> > see following comment execCurrentOf():
> >
> >         /*
> >          * This table didn't produce the cursor's current row; some other
> >          * inheritance child of the same parent must have.  Signal caller to
> >          * do nothing on this table.
> >          */
>
> Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
> would fail even with traditional inheritance:
>
> drop table if exists p cascade;
> create table p (a int);
> create table c (check (a = 2)) inherits (p);
> insert into p values (1);
> insert into c values (2);
> begin;
> declare c cursor for select * from p where a = 1;
> fetch c;
> update p set a = a where current of c;
> ERROR:  cursor "c" is not a simply updatable scan of table "c"
> ROLLBACK
>
> When there are no RowMarks to use because no FOR SHARE/UPDATE clause
> was specified when declaring the cursor, execCurrentOf() tries to find
> the cursor's current table by looking up its Scan node in the plan
> tree but will not find it if it was excluded in the cursor's query.
>
> With FOR SHARE/UPDATE, it seems to work because the planner delivers
> the RowMarks of all the children irrespective of whether or not they
> are present in the plan tree itself (something I had complained about
> in past [1]).  execCurrentOf() doesn't complain as long as there is a
> RowMark present even if it's never used.  For partitioning, the
> planner doesn't make RowMarks for pruned partitions, so
> execCurrentOf() can't find one if it's passed a pruned partition's
> oid.

I am missing something in this explanation. WHERE CURRENT OF works on
the row that was last fetched from a cursor. How could a pruned
partition's row be fetched and thus cause this error.
--
Best Wishes,
Ashutosh Bapat


Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Amit Langote
On Wed, May 27, 2020 at 9:11 PM Ashutosh Bapat
<[hidden email]> wrote:

> On Wed, May 27, 2020 at 12:53 PM Amit Langote <[hidden email]> wrote:
> > On Fri, May 22, 2020 at 9:09 PM amul sul <[hidden email]> wrote:
> > > I tried similar things on inherit partitioning as follow and that looks fine:
> > >
> > > DROP TABLE tbl;
> > > CREATE TABLE tbl (c1 INT,c2 TEXT);
> > > CREATE TABLE tbl_null(check (c1 is NULL)) INHERITS (tbl);
> > > CREATE TABLE tbl_1 (check (c1 > 0 and c1 < 4)) INHERITS (tbl);
> > > INSERT INTO tbl_1 VALUES(generate_series(1,3));
> > >
> > > postgres=# SELECT func(10);
> > >  func
> > > ------
> > >    10
> > > (1 row)
> > >
> > > On looking further for declarative partition, I found that issue happens only if
> > > the partitioning pruning enabled, see this:
> > >
> > > -- Execute on original set of test case.
> > > postgres=# ALTER FUNCTION func SET enable_partition_pruning to off;
> > > ALTER FUNCTION
> > >
> > > postgres=# SELECT func(10);
> > >  func
> > > ------
> > >    10
> > > (1 row)
> > >
> > > I think we need some indication in execCurrentOf() to skip error if the relation
> > > is pruned.  Something like that we already doing for inheriting partitioning,
> > > see following comment execCurrentOf():
> > >
> > >         /*
> > >          * This table didn't produce the cursor's current row; some other
> > >          * inheritance child of the same parent must have.  Signal caller to
> > >          * do nothing on this table.
> > >          */
> >
> > Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
> > would fail even with traditional inheritance:
> >
> > drop table if exists p cascade;
> > create table p (a int);
> > create table c (check (a = 2)) inherits (p);
> > insert into p values (1);
> > insert into c values (2);
> > begin;
> > declare c cursor for select * from p where a = 1;
> > fetch c;
> > update p set a = a where current of c;
> > ERROR:  cursor "c" is not a simply updatable scan of table "c"
> > ROLLBACK
> >
> > When there are no RowMarks to use because no FOR SHARE/UPDATE clause
> > was specified when declaring the cursor, execCurrentOf() tries to find
> > the cursor's current table by looking up its Scan node in the plan
> > tree but will not find it if it was excluded in the cursor's query.
> >
> > With FOR SHARE/UPDATE, it seems to work because the planner delivers
> > the RowMarks of all the children irrespective of whether or not they
> > are present in the plan tree itself (something I had complained about
> > in past [1]).  execCurrentOf() doesn't complain as long as there is a
> > RowMark present even if it's never used.  For partitioning, the
> > planner doesn't make RowMarks for pruned partitions, so
> > execCurrentOf() can't find one if it's passed a pruned partition's
> > oid.
>
> I am missing something in this explanation. WHERE CURRENT OF works on
> the row that was last fetched from a cursor. How could a pruned
> partition's row be fetched and thus cause this error.

So in Rajkumar's example, the cursor is declared as:

CURSOR IS SELECT * FROM tbl WHERE c1< 5 FOR UPDATE;

and the WHERE CURRENT OF query is this:

 UPDATE tbl SET c2='aa' WHERE CURRENT OF cur;

You can see that the UPDATE will scan all partitions, whereas the
cursor's query does not.

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

amul sul
In reply to this post by Amit Langote
On Wed, May 27, 2020 at 12:53 PM Amit Langote <[hidden email]> wrote:
On Fri, May 22, 2020 at 9:09 PM amul sul <[hidden email]> wrote:
> I tried similar things on inherit partitioning as follow and that looks fine:
>
> DROP TABLE tbl;
> CREATE TABLE tbl (c1 INT,c2 TEXT);
> CREATE TABLE tbl_null(check (c1 is NULL)) INHERITS (tbl);
> CREATE TABLE tbl_1 (check (c1 > 0 and c1 < 4)) INHERITS (tbl);
> INSERT INTO tbl_1 VALUES(generate_series(1,3));
>
> postgres=# SELECT func(10);
>  func
> ------
>    10
> (1 row)
>
> On looking further for declarative partition, I found that issue happens only if
> the partitioning pruning enabled, see this:
>
> -- Execute on original set of test case.
> postgres=# ALTER FUNCTION func SET enable_partition_pruning to off;
> ALTER FUNCTION
>
> postgres=# SELECT func(10);
>  func
> ------
>    10
> (1 row)
>
> I think we need some indication in execCurrentOf() to skip error if the relation
> is pruned.  Something like that we already doing for inheriting partitioning,
> see following comment execCurrentOf():
>
>         /*
>          * This table didn't produce the cursor's current row; some other
>          * inheritance child of the same parent must have.  Signal caller to
>          * do nothing on this table.
>          */

Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
would fail even with traditional inheritance:

drop table if exists p cascade;
create table p (a int);
create table c (check (a = 2)) inherits (p);
insert into p values (1);
insert into c values (2);
begin;
declare c cursor for select * from p where a = 1;
fetch c;
update p set a = a where current of c;
ERROR:  cursor "c" is not a simply updatable scan of table "c"
ROLLBACK


I am not sure I understood the point, you'll see the same error with declarative
partitioning as well. 
 
When there are no RowMarks to use because no FOR SHARE/UPDATE clause
was specified when declaring the cursor, execCurrentOf() tries to find
the cursor's current table by looking up its Scan node in the plan
tree but will not find it if it was excluded in the cursor's query.

With FOR SHARE/UPDATE, it seems to work because the planner delivers
the RowMarks of all the children irrespective of whether or not they
are present in the plan tree itself (something I had complained about
in past [1]).  execCurrentOf() doesn't complain as long as there is a
RowMark present even if it's never used.  For partitioning, the
planner doesn't make RowMarks for pruned partitions, so
execCurrentOf() can't find one if it's passed a pruned partition's
oid.


Right.
 
I don't see a way to avoid these errors.  How does execCurrentOf()
distinguish a table that could *never* be present in a cursor from a
table that could be had it not been pruned/excluded?  If it can do
that, then give an error for the former and return false for the
latter.

Yeah. I haven't thought much about this; I was thinking initially just to skip
error by assuming that the table that we are looking might have pruned, but I am
not sure how bad or good approach it is. 


I guess the workaround is to declare the cursor such that no
partitions/children are pruned/excluded.


Disabling pruning as well -- at-least for the statement or function. 

Regards,
Amul


--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com

[1] https://www.postgresql.org/message-id/468c85d9-540e-66a2-1dde-fec2b741e688%40lab.ntt.co.jp
Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Amit Langote
On Thu, May 28, 2020 at 1:36 PM amul sul <[hidden email]> wrote:

> On Wed, May 27, 2020 at 12:53 PM Amit Langote <[hidden email]> wrote:
>> Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
>> would fail even with traditional inheritance:
>>
>> drop table if exists p cascade;
>> create table p (a int);
>> create table c (check (a = 2)) inherits (p);
>> insert into p values (1);
>> insert into c values (2);
>> begin;
>> declare c cursor for select * from p where a = 1;
>> fetch c;
>> update p set a = a where current of c;
>> ERROR:  cursor "c" is not a simply updatable scan of table "c"
>> ROLLBACK
>>
>
> I am not sure I understood the point, you'll see the same error with declarative
> partitioning as well.

My point is that if a table is not present in the cursor's plan, there
is no way for CURRENT OF to access it.  Giving an error in that case
seems justified.

OTOH, when the CURRENT OF implementation has RowMarks to look at, it
avoids the error for traditional inheritance children due their
inactive RowMarks being present in the cursor's PlannedStmt.  I think
that's only by accident though.

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Amit Langote
In reply to this post by amul sul
On Thu, May 28, 2020 at 1:36 PM amul sul <[hidden email]> wrote:
> On Wed, May 27, 2020 at 12:53 PM Amit Langote <[hidden email]> wrote:
>> I guess the workaround is to declare the cursor such that no
>> partitions/children are pruned/excluded.
>
> Disabling pruning as well -- at-least for the statement or function.

Now *that* is actually a workaround to tell a customer. :)

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

amul sul
In reply to this post by Amit Langote


On Thu, May 28, 2020 at 3:06 PM Amit Langote <[hidden email]> wrote:
On Thu, May 28, 2020 at 1:36 PM amul sul <[hidden email]> wrote:
> On Wed, May 27, 2020 at 12:53 PM Amit Langote <[hidden email]> wrote:
>> Actually, if you declare the cursor without FOR SHARE/UPDATE, the case
>> would fail even with traditional inheritance:
>>
>> drop table if exists p cascade;
>> create table p (a int);
>> create table c (check (a = 2)) inherits (p);
>> insert into p values (1);
>> insert into c values (2);
>> begin;
>> declare c cursor for select * from p where a = 1;
>> fetch c;
>> update p set a = a where current of c;
>> ERROR:  cursor "c" is not a simply updatable scan of table "c"
>> ROLLBACK
>>
>
> I am not sure I understood the point, you'll see the same error with declarative
> partitioning as well.

My point is that if a table is not present in the cursor's plan, there
is no way for CURRENT OF to access it.  Giving an error in that case
seems justified.

OTOH, when the CURRENT OF implementation has RowMarks to look at, it
avoids the error for traditional inheritance children due their
inactive RowMarks being present in the cursor's PlannedStmt.  I think
that's only by accident though.

Yeah, make sense, thank you.

Regards,
Amul 
Reply | Threaded
Open this post in threaded view
|

Re: Getting ERROR with FOR UPDATE/SHARE for partitioned table.

Ashutosh Bapat-2
In reply to this post by Amit Langote
On Wed, May 27, 2020 at 6:51 PM Amit Langote <[hidden email]> wrote:

>
> So in Rajkumar's example, the cursor is declared as:
>
> CURSOR IS SELECT * FROM tbl WHERE c1< 5 FOR UPDATE;
>
> and the WHERE CURRENT OF query is this:
>
>  UPDATE tbl SET c2='aa' WHERE CURRENT OF cur;

Thanks for the clarification. So it looks like we expand UPDATE on
partitioned table to UPDATE on each partition (inheritance_planner for
DML) and then execute each of those. If CURRENT OF were to save the
table oid or something we could run the UPDATE only on that partition.
I am possibly shooting in dark, but this puzzles me. And it looks like
we can cause wrong rows to be updated in non-partition inheritance
where the ctids match?

--
Best Wishes,
Ashutosh Bapat