not fully correct error message

Started by Pavel Stehule2 months ago12 messages
Jump to latest
#1Pavel Stehule
pavel.stehule@gmail.com

Hi

I tested one use case, and maybe I found little bit possible error message

create procedure test()
as $$
begin
vacuum;
end;
$$ language plpgsql;

(2026-01-01 08:04:05) postgres=# call test();
ERROR: 25001: VACUUM cannot be executed from a function
CONTEXT: SQL statement "vacuum"
PL/pgSQL function test() line 3 at SQL statement
LOCATION: PreventInTransactionBlock, xact.c:3695
(2026-01-01 08:09:18) postgres=#

should be "VACUUM cannot be executed from a function or a procedure"
instead ?

Regards

Pavel

#2jian he
jian.universality@gmail.com
In reply to: Pavel Stehule (#1)
Re: not fully correct error message

On Thu, Jan 1, 2026 at 3:10 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hi

I tested one use case, and maybe I found little bit possible error message

create procedure test()
as $$
begin
vacuum;
end;
$$ language plpgsql;

(2026-01-01 08:04:05) postgres=# call test();
ERROR: 25001: VACUUM cannot be executed from a function
CONTEXT: SQL statement "vacuum"
PL/pgSQL function test() line 3 at SQL statement
LOCATION: PreventInTransactionBlock, xact.c:3695
(2026-01-01 08:09:18) postgres=#

should be "VACUUM cannot be executed from a function or a procedure" instead ?

hi.
"VACUUM cannot be executed from a function or a procedure"
looks good to me.

similarly, in ExecWaitStmt we have:
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAIT FOR must be only called without an active
or registered snapshot"),
errdetail("WAIT FOR cannot be executed from a function
or a procedure or within a transaction with an isolation level higher
than READ COMMITTED."));

PreventInTransactionBlock is used in so many places, but this error message:
``
(errmsg("%s cannot be executed from a function", stmtType)));
``
only appears once in the regress tests.
maybe we can add some dummy tests for it.

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: jian he (#2)
Re: not fully correct error message

Hi

čt 1. 1. 2026 v 11:05 odesílatel jian he <jian.universality@gmail.com>
napsal:

On Thu, Jan 1, 2026 at 3:10 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Hi

I tested one use case, and maybe I found little bit possible error

message

create procedure test()
as $$
begin
vacuum;
end;
$$ language plpgsql;

(2026-01-01 08:04:05) postgres=# call test();
ERROR: 25001: VACUUM cannot be executed from a function
CONTEXT: SQL statement "vacuum"
PL/pgSQL function test() line 3 at SQL statement
LOCATION: PreventInTransactionBlock, xact.c:3695
(2026-01-01 08:09:18) postgres=#

should be "VACUUM cannot be executed from a function or a procedure"

instead ?

hi.
"VACUUM cannot be executed from a function or a procedure"
looks good to me.

similarly, in ExecWaitStmt we have:
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAIT FOR must be only called without an active
or registered snapshot"),
errdetail("WAIT FOR cannot be executed from a function
or a procedure or within a transaction with an isolation level higher
than READ COMMITTED."));

PreventInTransactionBlock is used in so many places, but this error
message:
``
(errmsg("%s cannot be executed from a function", stmtType)));
``
only appears once in the regress tests.
maybe we can add some dummy tests for it.

here is a patch (with small regress test)

Regards

Pavel

Attachments:

0001-VACUUM-cannot-be-executed-inside-a-function-or-a-pro.patchtext/x-patch; charset=US-ASCII; name=0001-VACUUM-cannot-be-executed-inside-a-function-or-a-pro.patchDownload+28-2
#4Andreas Karlsson
andreas.karlsson@percona.com
In reply to: Pavel Stehule (#3)
Re: not fully correct error message

On 1/3/26 7:34 AM, Pavel Stehule wrote:

here is a patch (with small regress test)

Looks like a good change to me since the obvious question many people
would ask themselves after seeing this error is if running it in a
procedure would work instead and it does not. And this error message
changes saves those people time which is a great thing.

Tried out this patch and it worked and, as expected, also improved the
error for other commands like CREATE DATABASE and REINDEX CONCURRENTLY
so when this is commit the commit message should make it clear this is
not just about VACUUM.

A small suggestion is to change the message from:

"%s cannot be executed from a function or a procedure"

to:

"%s cannot be executed from a function or procedure"

I am not a native speaker but I think the second flows better.

Andreas

#5Marcos Pegoraro
marcos@f10.com.br
In reply to: Pavel Stehule (#3)
Re: not fully correct error message

Em sáb., 3 de jan. de 2026 às 03:35, Pavel Stehule <pavel.stehule@gmail.com>
escreveu:

here is a patch (with small regress test)

An anonymous block doesn't accept vacuum too.
Wouldn't it be better to specify what kind of block you are running instead
of
function, procedure or anonymous block ?

regards
Marcos

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Marcos Pegoraro (#5)
Re: not fully correct error message

so 3. 1. 2026 v 13:23 odesílatel Marcos Pegoraro <marcos@f10.com.br> napsal:

Em sáb., 3 de jan. de 2026 às 03:35, Pavel Stehule <
pavel.stehule@gmail.com> escreveu:

here is a patch (with small regress test)

An anonymous block doesn't accept vacuum too.
Wouldn't it be better to specify what kind of block you are running
instead of
function, procedure or anonymous block ?

It is correct, but maybe too long.

Generally, there is a term "routine" as a synonym for "function, procedure
or any stored procedure code", but I am not sure how much is accepted by
the community

Show quoted text

regards
Marcos

#7Andreas Karlsson
andreas.karlsson@percona.com
In reply to: Marcos Pegoraro (#5)
Re: not fully correct error message

On 1/3/26 1:22 PM, Marcos Pegoraro wrote:

Em sáb., 3 de jan. de 2026 às 03:35, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>> escreveu:

here is a patch (with small regress test)

An anonymous block doesn't accept vacuum too.
Wouldn't it be better to specify what kind of block you are running
instead of
function, procedure or anonymous block ?

Maybe out of some kind of correctness, but it seems less useful to me
since the obvious question an end user would ask after trying to run
VACUUM in a function is if they can do so in a procedure instead so we
may as well tell them right away.

A potential third option would be to take your solution but to add a
HINT about that it needs to run as a top-level statement outside any
transactions, but I kinda liked how simple the original patch was.

Andreas

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andreas Karlsson (#7)
Re: not fully correct error message

Andreas Karlsson <andreas@proxel.se> writes:

On 1/3/26 1:22 PM, Marcos Pegoraro wrote:

Em sáb., 3 de jan. de 2026 às 03:35, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>> escreveu:

here is a patch (with small regress test)

An anonymous block doesn't accept vacuum too.
Wouldn't it be better to specify what kind of block you are running
instead of function, procedure or anonymous block ?

I don't think PreventInTransactionBlock has ready access to that
information.

A potential third option would be to take your solution but to add a
HINT about that it needs to run as a top-level statement outside any
transactions, but I kinda liked how simple the original patch was.

Yeah, I like just adding "or procedure" and calling it good.
I do not think we need a regression test, either ...

Poking around, I also found this:

src/backend/commands/wait.c: errdetail("WAIT FOR cannot be executed from a function or a procedure or within a transaction with an isolation level higher than READ COMMITTED."));

which is also not great grammar. What do you think of "WAIT FOR
cannot be executed from a function or procedure, nor within a
transaction with an isolation level higher than READ COMMITTED." ?

regards, tom lane

#9Andreas Karlsson
andreas.karlsson@percona.com
In reply to: Tom Lane (#8)
Re: not fully correct error message

On 1/3/26 7:03 PM, Tom Lane wrote:

Andreas Karlsson <andreas@proxel.se> writes:

A potential third option would be to take your solution but to add a
HINT about that it needs to run as a top-level statement outside any
transactions, but I kinda liked how simple the original patch was.

Yeah, I like just adding "or procedure" and calling it good.
I do not think we need a regression test, either ...

Yeah, let's keep it simple.

Poking around, I also found this:

src/backend/commands/wait.c: errdetail("WAIT FOR cannot be executed from a function or a procedure or within a transaction with an isolation level higher than READ COMMITTED."));

which is also not great grammar. What do you think of "WAIT FOR
cannot be executed from a function or procedure, nor within a
transaction with an isolation level higher than READ COMMITTED." ?

Much better!

Andreas

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andreas Karlsson (#9)
Re: not fully correct error message

Andreas Karlsson <andreas@proxel.se> writes:

On 1/3/26 7:03 PM, Tom Lane wrote:

Yeah, I like just adding "or procedure" and calling it good.
I do not think we need a regression test, either ...

Yeah, let's keep it simple.

Poking around, I also found this:
src/backend/commands/wait.c: errdetail("WAIT FOR cannot be executed from a function or a procedure or within a transaction with an isolation level higher than READ COMMITTED."));
which is also not great grammar. What do you think of "WAIT FOR
cannot be executed from a function or procedure, nor within a
transaction with an isolation level higher than READ COMMITTED." ?

Much better!

Putting that all together, and fixing affected regression tests
(yes, this code was covered already), I get the attached.

regards, tom lane

Attachments:

v2-improve-vacuum-error-message.patchtext/x-diff; charset=us-ascii; name=v2-improve-vacuum-error-message.patchDownload+7-6
#11Andreas Karlsson
andreas.karlsson@percona.com
In reply to: Tom Lane (#10)
Re: not fully correct error message

On 1/3/26 8:16 PM, Tom Lane wrote:

Putting that all together, and fixing affected regression tests
(yes, this code was covered already), I get the attached.

That looks good to me.

Andreas

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andreas Karlsson (#11)
Re: not fully correct error message

Andreas Karlsson <andreas@proxel.se> writes:

On 1/3/26 8:16 PM, Tom Lane wrote:

Putting that all together, and fixing affected regression tests
(yes, this code was covered already), I get the attached.

That looks good to me.

Pushed.

regards, tom lane