Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

2017-10-26 Thread Robert Haas
On Mon, Oct 23, 2017 at 10:27 AM, Andres Freund  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


Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

2017-10-23 Thread Andres Freund
On 2017-10-22 23:04:50 -0400, Tom Lane wrote:
> John Lumby  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


Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

2017-10-23 Thread Andres Freund
On 2017-10-23 16:16:10 +0800, Craig Ringer wrote:
> On 23 October 2017 at 08:30, John Lumby  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


Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

2017-10-23 Thread Craig Ringer
On 23 October 2017 at 16:16, Craig Ringer  wrote:
> On 23 October 2017 at 08:30, John Lumby  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


Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

2017-10-23 Thread Craig Ringer
On 23 October 2017 at 08:30, John Lumby  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


Re: [HACKERS] legitimacy of using PG_TRY , PG_CATCH , PG_END_TRY in C function

2017-10-22 Thread Tom Lane
John Lumby  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