Document that PG_TRY block cannot have a return statement

Started by Serpentover 2 years ago5 messages
#1Serpent
serpent7776@gmail.com
1 attachment(s)

Hi,

I created a tiny patch that documents that the code block following
PG_TRY() cannot have any return statement.

Please CC me, as I'm not subscribed to this list.

Attachments:

0001-note-no-return-in-pg_try.patchtext/x-patch; charset=US-ASCII; name=0001-note-no-return-in-pg_try.patchDownload
commit 1968e53b9a649691fbedbdc059e2e933aa2b471b
Author: Serpent7776 <Serpent7776@gmail.com>
Date:   Tue Sep 12 14:38:09 2023 +0200

    Document that PG_TRY block cannot have a return statement
    
    Document the fact that the block of code that follows `PG_TRY()` cannot
    have any form of return statement, because `PG_TRY()` modifies global
    state that is later restored in `PG_CATCH()` or `PG_FINALLY()`.

diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 4a9562fdaa..f81b94b054 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -352,6 +352,8 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
  * You cannot use both PG_CATCH() and PG_FINALLY() in the same
  * PG_TRY()/PG_END_TRY() block.
  *
+ * There cannot be a return statement in the block of code following PG_TRY().
+ *
  * Note: while the system will correctly propagate any new ereport(ERROR)
  * occurring in the recovery section, there is a small limit on the number
  * of levels this will work for.  It's best to keep the error recovery
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Serpent (#1)
Re: Document that PG_TRY block cannot have a return statement

Serpent <serpent7776@gmail.com> writes:

I created a tiny patch that documents that the code block following
PG_TRY() cannot have any return statement.

AFAIK, this is wrong. The actual requirement is already stated
in the comment:

* ... The error recovery code
* can either do PG_RE_THROW to propagate the error outwards, or do a
* (sub)transaction abort.

regards, tom lane

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Serpent (#1)
Re: Document that PG_TRY block cannot have a return statement

Serpent <serpent7776@gmail.com> writes:

I'm talking about this part:

PG_TRY();
{
... code that might throw ereport(ERROR) ...
}

Ah. Your phrasing needs work for clarity then. Also, "return"
is hardly the only way to break it; break, continue, or goto
leading out of the PG_TRY are other possibilities. Maybe more
like "The XXX code must exit normally (by control reaching
the end) if it does not throw ereport(ERROR)." Not quite sure
what to use for XXX.

regards, tom lane

#4Serpent
serpent7776@gmail.com
In reply to: Tom Lane (#3)
1 attachment(s)
Re: Document that PG_TRY block cannot have a return statement

Hi,

What about this wording:

The code that might throw ereport(ERROR) cannot contain any non local
control flow other than ereport(ERROR) e.g.: return, goto, break, continue.
In other words once PG_TRY() is executed, either PG_CATCH() or PG_FINALLY()
must be executed as well.

I used 'code that might throw ereport(ERROR)' for XXX since this is what's
used earlier in the comment.

On Tue, 12 Sept 2023 at 17:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Show quoted text

Serpent <serpent7776@gmail.com> writes:

I'm talking about this part:

PG_TRY();
{
... code that might throw ereport(ERROR) ...
}

Ah. Your phrasing needs work for clarity then. Also, "return"
is hardly the only way to break it; break, continue, or goto
leading out of the PG_TRY are other possibilities. Maybe more
like "The XXX code must exit normally (by control reaching
the end) if it does not throw ereport(ERROR)." Not quite sure
what to use for XXX.

regards, tom lane

Attachments:

0002-note-no-return-in-pg_try.patchtext/x-patch; charset=US-ASCII; name=0002-note-no-return-in-pg_try.patchDownload
commit d077d0473378271d8cc4c0685b22ec61c9bce3f4
Author: Serpent7776 <Serpent7776@gmail.com>
Date:   Tue Sep 12 14:38:09 2023 +0200

    Document that PG_TRY block cannot have a return statement
    
    Document the fact that the block of code that follows `PG_TRY()` cannot
    have any form of return statement, because `PG_TRY()` modifies global
    state that is later restored in `PG_CATCH()` or `PG_FINALLY()`.

diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 4a9562fdaa..1bcd146117 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -352,6 +352,11 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
  * You cannot use both PG_CATCH() and PG_FINALLY() in the same
  * PG_TRY()/PG_END_TRY() block.
  *
+ * The code that might throw ereport(ERROR) cannot contain any non local
+ * control flow other than ereport(ERROR) e.g.: return, goto, break, continue.
+ * In other words once PG_TRY() is executed, either PG_CATCH() or PG_FINALLY()
+ * must be executed as well.
+ *
  * Note: while the system will correctly propagate any new ereport(ERROR)
  * occurring in the recovery section, there is a small limit on the number
  * of levels this will work for.  It's best to keep the error recovery
#5Xiaoran Wang
fanfuxiaoran@gmail.com
In reply to: Serpent (#4)
Re: Document that PG_TRY block cannot have a return statement

LGTM!

Serpent <serpent7776@gmail.com> 于2024年8月15日周四 15:01写道:

Hi,

What about this wording:

The code that might throw ereport(ERROR) cannot contain any non local
control flow other than ereport(ERROR) e.g.: return, goto, break, continue.
In other words once PG_TRY() is executed, either PG_CATCH() or
PG_FINALLY() must be executed as well.

I used 'code that might throw ereport(ERROR)' for XXX since this is what's
used earlier in the comment.

On Tue, 12 Sept 2023 at 17:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Serpent <serpent7776@gmail.com> writes:

I'm talking about this part:

PG_TRY();
{
... code that might throw ereport(ERROR) ...
}

Ah. Your phrasing needs work for clarity then. Also, "return"
is hardly the only way to break it; break, continue, or goto
leading out of the PG_TRY are other possibilities. Maybe more
like "The XXX code must exit normally (by control reaching
the end) if it does not throw ereport(ERROR)." Not quite sure
what to use for XXX.

regards, tom lane

--
Best regards !
Xiaoran Wang