Outdated comments around HandleFunctionRequest

On 04/05/2017 04:05 AM, Andres Freund wrote:

PostgresMain() has the following blurb for fastpath functions:

/*
* Note: we may at this point be inside an aborted
* transaction. We can't throw error for that until we've
* finished reading the function-call message, so
* HandleFunctionRequest() must check for it after doing so.
* Be careful not to do anything that assumes we're inside a
* valid transaction here.
*/
and in HandleFunctionRequest() there's:

* INPUT:
* In protocol version 3, postgres.c has already read the message body
* and will pass it in msgBuf.
* In old protocol, the passed msgBuf is empty and we must read the
* message here.

which is not true anymore. Followed by:

/*
* Now that we've eaten the input message, check to see if we actually
* want to do the function call or not. It's now safe to ereport(); we
* won't lose sync with the frontend.
*/

which is also not really meaningful, because there's no previous code in
the function.

You're right, I missed those comments in commit 2b3a8b20c2.

In fact, HandleFunctionRequest() now always return 0, so we can also
remove the dead code to check the return value from the caller. Barring
objections, I'll commit the attached to do that and fix the comments.

- Heikki

On 04/05/2017 10:58 AM, Heikki Linnakangas wrote:

On 04/05/2017 04:05 AM, Andres Freund wrote:

PostgresMain() has the following blurb for fastpath functions:

/*
* Note: we may at this point be inside an aborted
* transaction. We can't throw error for that until we've
* finished reading the function-call message, so
* HandleFunctionRequest() must check for it after doing so.
* Be careful not to do anything that assumes we're inside a
* valid transaction here.
*/
and in HandleFunctionRequest() there's:

* INPUT:
* In protocol version 3, postgres.c has already read the message body
* and will pass it in msgBuf.
* In old protocol, the passed msgBuf is empty and we must read the
* message here.

which is not true anymore. Followed by:

/*
* Now that we've eaten the input message, check to see if we actually
* want to do the function call or not. It's now safe to ereport(); we
* won't lose sync with the frontend.
*/

which is also not really meaningful, because there's no previous code in
the function.

You're right, I missed those comments in commit 2b3a8b20c2.

In fact, HandleFunctionRequest() now always return 0, so we can also
remove the dead code to check the return value from the caller. Barring
objections, I'll commit the attached to do that and fix the comments.

Committed, thanks!

- Heikki

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