Обсуждение: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

I have a C function (a trigger function) which uses the PG_TRY() 
construct to handle certain ERROR conditions.
One example is where invoked as INSTEAD OF INSERT into a view.  It 
PG_TRYs INSERT into the real base table,
but this table may not yet exist  (it is a partitioned child of an 
inheritance parent).
If the error is  ERRCODE_UNDEFINED_TABLE,  then the CATCH issues 
FlushErrorState() and returns to caller who CREATes the table and 
re-issues the insert.
All works perfectly (on every release of 9.x).

But in a different part of the same trigger function,   there is a 
PG_TRY , PG_CATCH of a CREATE INDEX,
which sometimes hits error "relation already exists" (i.e. index with 
same name indexing a different table).
The CATCH issues FlushErrorState() and returns to caller who ignores the 
error.
All works but not perfectly --  at COMMIT,  resource_owner issues  
relcache reference leak messages about relation scans not closed
and also about  snapshot still active.     I guess that the CREATE has 
switched resource_owner and pushed a snapshot,  but I did not
debug in detail.

Those are just examples,  but my question is whether use of this 
construct is legal in a C function
(always?   sometimes  e,g only for DML but not DDL?  never?),
and if not,   then how can one legally catch ERRORs like this in a C 
function?
(Yes I realise I could write selects of pg_class to avoid these two 
particular cases rather than the try-and-fail approach
but I would like to understand the general rules where it may be much 
harder to try avoidance)

Cheers,  John


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

John Lumby <johnlumby@hotmail.com> writes:
> I have a C function (a trigger function) which uses the PG_TRY() 
> construct to handle certain ERROR conditions.
> One example is where invoked as INSTEAD OF INSERT into a view.  It 
> PG_TRYs INSERT into the real base table,
> but this table may not yet exist  (it is a partitioned child of an 
> inheritance parent).
> If the error is  ERRCODE_UNDEFINED_TABLE,  then the CATCH issues 
> FlushErrorState() and returns to caller who CREATes the table and 
> re-issues the insert.
> All works perfectly (on every release of 9.x).

If it works, it's only because you didn't try very hard to break it.
In general you can't catch and ignore errors without a full-fledged
subtransaction --- BeginInternalSubTransaction, then either
ReleaseCurrentSubTransaction or RollbackAndReleaseCurrentSubTransaction,
not just FlushErrorState.  See e.g. plpgpsql's exec_stmt_block.

There may well be specific scenarios where an error gets thrown without
having done anything that requires transaction cleanup.  But when you
hit a scenario where that's not true, or when a scenario that used to
not require cleanup now does, nobody is going to consider that a PG bug.
        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 23 October 2017 at 08:30, John Lumby <johnlumby@hotmail.com> wrote:

> All works but not perfectly --  at COMMIT,  resource_owner issues
> relcache reference leak messages about relation scans not closed
> and also about  snapshot still active.     I guess that the CREATE has
> switched resource_owner and pushed a snapshot,  but I did not
> debug in detail.

A lot more work is required than what's done pg PG_CATCH to return to
a queryable state. I've been down this path myself, and it's not fun.

Take a look at all the extra work done on the error handling path in
PostgresMain.

At some point I'd really like to expose that in a more general way so
it can be used from background workers. Right now AFAICS most
background workers have to cope with errors with a proc_exit(1) and
getting restarted to try again. Not ideal.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 23 October 2017 at 16:16, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 23 October 2017 at 08:30, John Lumby <johnlumby@hotmail.com> wrote:
>
>> All works but not perfectly --  at COMMIT,  resource_owner issues
>> relcache reference leak messages about relation scans not closed
>> and also about  snapshot still active.     I guess that the CREATE has
>> switched resource_owner and pushed a snapshot,  but I did not
>> debug in detail.
>
> A lot more work is required than what's done pg PG_CATCH to return to
> a queryable state. I've been down this path myself, and it's not fun.

Ignore me, Tom's example is probably more relevant to you since it
applies to subtransactions, not top-level query state.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-10-23 16:16:10 +0800, Craig Ringer wrote:
> On 23 October 2017 at 08:30, John Lumby <johnlumby@hotmail.com> wrote:
> 
> > All works but not perfectly --  at COMMIT,  resource_owner issues
> > relcache reference leak messages about relation scans not closed
> > and also about  snapshot still active.     I guess that the CREATE has
> > switched resource_owner and pushed a snapshot,  but I did not
> > debug in detail.
> 
> A lot more work is required than what's done pg PG_CATCH to return to
> a queryable state. I've been down this path myself, and it's not fun.
> 
> Take a look at all the extra work done on the error handling path in
> PostgresMain.

That seems quite misleading - that's *not* what needs to be done
to catch an error inside a function. See Tom's response.


> At some point I'd really like to expose that in a more general way so
> it can be used from background workers. Right now AFAICS most
> background workers have to cope with errors with a proc_exit(1) and
> getting restarted to try again. Not ideal.

I agree that generalizing wouldn't be bad, but there's absolutely
nothing preventing you from handling errors in bgworkers without
restarting today.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-10-22 23:04:50 -0400, Tom Lane wrote:
> John Lumby <johnlumby@hotmail.com> writes:
> > I have a C function (a trigger function) which uses the PG_TRY() 
> > construct to handle certain ERROR conditions.
> > One example is where invoked as INSTEAD OF INSERT into a view.  It 
> > PG_TRYs INSERT into the real base table,
> > but this table may not yet exist  (it is a partitioned child of an 
> > inheritance parent).
> > If the error is  ERRCODE_UNDEFINED_TABLE,  then the CATCH issues 
> > FlushErrorState() and returns to caller who CREATes the table and 
> > re-issues the insert.
> > All works perfectly (on every release of 9.x).
> 
> If it works, it's only because you didn't try very hard to break it.
> In general you can't catch and ignore errors without a full-fledged
> subtransaction --- BeginInternalSubTransaction, then either
> ReleaseCurrentSubTransaction or RollbackAndReleaseCurrentSubTransaction,
> not just FlushErrorState.  See e.g. plpgpsql's exec_stmt_block.
> 
> There may well be specific scenarios where an error gets thrown without
> having done anything that requires transaction cleanup.  But when you
> hit a scenario where that's not true, or when a scenario that used to
> not require cleanup now does, nobody is going to consider that a PG bug.

It'd probably be a good idea to expand on this in the sgml docs. This
has confused quite anumber of people...

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Oct 23, 2017 at 10:27 AM, Andres Freund <andres@anarazel.de> wrote:
> It'd probably be a good idea to expand on this in the sgml docs. This
> has confused quite anumber of people...

That's a good idea.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Oct 23, 2017 at 01:27:43AM -0700, Andres Freund wrote:
> On 2017-10-22 23:04:50 -0400, Tom Lane wrote:
> > John Lumby <johnlumby@hotmail.com> writes:
> > > I have a C function (a trigger function) which uses the PG_TRY() 
> > > construct to handle certain ERROR conditions.
> > > One example is where invoked as INSTEAD OF INSERT into a view.  It 
> > > PG_TRYs INSERT into the real base table,
> > > but this table may not yet exist  (it is a partitioned child of an 
> > > inheritance parent).
> > > If the error is  ERRCODE_UNDEFINED_TABLE,  then the CATCH issues 
> > > FlushErrorState() and returns to caller who CREATes the table and 
> > > re-issues the insert.
> > > All works perfectly (on every release of 9.x).
> > 
> > If it works, it's only because you didn't try very hard to break it.
> > In general you can't catch and ignore errors without a full-fledged
> > subtransaction --- BeginInternalSubTransaction, then either
> > ReleaseCurrentSubTransaction or RollbackAndReleaseCurrentSubTransaction,
> > not just FlushErrorState.  See e.g. plpgpsql's exec_stmt_block.
> > 
> > There may well be specific scenarios where an error gets thrown without
> > having done anything that requires transaction cleanup.  But when you
> > hit a scenario where that's not true, or when a scenario that used to
> > not require cleanup now does, nobody is going to consider that a PG bug.
> 
> It'd probably be a good idea to expand on this in the sgml docs. This
> has confused quite anumber of people...

I know this is from 2017, but where would we document this?  I don't see
PG_TRY/PG_CATCH mentioned in the SGML docs at all.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.