okbob/plpgsql_check

Test failure with PG15: missing "error:2D000:5:COMMIT:cannot commit while a subtransaction is active" output

df7cb opened this issue · 13 comments

df7cb commented

I see there is a specific test file for PG15, but it doesn't match:

14:59:57 **** regression.diffs ****
14:59:57 diff -U3 /<<PKGBUILDDIR>>/expected/plpgsql_check_active-15.out /<<PKGBUILDDIR>>/results/plpgsql_check_active-15.out
14:59:57 --- /<<PKGBUILDDIR>>/expected/plpgsql_check_active-15.out	2022-05-16 05:26:38.000000000 +0000
14:59:57 +++ /<<PKGBUILDDIR>>/results/plpgsql_check_active-15.out	2022-05-20 12:59:56.991819484 +0000
14:59:57 @@ -527,10 +527,9 @@
14:59:57  end;
14:59:57  $$ language plpgsql;
14:59:57  select * from plpgsql_check_function('prtest'); --error
14:59:57 -                       plpgsql_check_function                        
14:59:57 ----------------------------------------------------------------------
14:59:57 - error:2D000:5:COMMIT:cannot commit while a subtransaction is active
14:59:57 -(1 row)
14:59:57 + plpgsql_check_function 
14:59:57 +------------------------
14:59:57 +(0 rows)
14:59:57  
14:59:57  create or replace procedure prtest()
14:59:57  as $$

Full build log: https://pgdgbuild.dus.dg-i.net/view/Binaries-Beta/job/plpgsql-check-binaries-beta/11/architecture=amd64,distribution=sid/console#console-section-8

okbob commented
df7cb commented

No error here:

$ psql
psql (15beta1 (Debian 15~beta1-1.pgdg+1))
Geben Sie »help« für Hilfe ein.

16:30 cbe@postgres =# create extension plpgsql_check ;
CREATE EXTENSION
Zeit: 9,077 ms
16:30 cbe@postgres =# create or replace procedure prtest()
16:31 cbe@postgres -# as $$
16:31 cbe@postgres $# begin
16:31 cbe@postgres $#   begin
16:31 cbe@postgres $#     begin
16:31 cbe@postgres $#       commit;
16:31 cbe@postgres $#     end;
16:31 cbe@postgres $#   end;
16:31 cbe@postgres $# exception when others then
16:31 cbe@postgres $#   raise;
16:31 cbe@postgres $# end;
16:31 cbe@postgres $# $$ language plpgsql;
CREATE PROCEDURE
Zeit: 0,705 ms
16:31 cbe@postgres =# select * from plpgsql_check_function('prtest');
 plpgsql_check_function 
────────────────────────
(0 Zeilen)

Zeit: 0,855 ms
okbob commented
okbob commented
rjuju commented

I could reproduce it locally on a standard pg15 debug install, not on a debian environment. One easy way to hit it (at least on my environment) is to check the function multiple times:

# select * from plpgsql_check_function('prtest');
                       plpgsql_check_function
---------------------------------------------------------------------
 error:2D000:5:COMMIT:cannot commit while a subtransaction is active
(1 row)

# select * from plpgsql_check_function('prtest');
 plpgsql_check_function
------------------------
(0 rows)

The problem is that after the 2nd call (and all of them afterwards), is_exception_handler is true for all PLpgSQL_stmt_stack_item in the function, so is_inside_protected_block() never returns true anymore. AFAICS, there's nowhere in the code where the stack item is explicitly initialized, so the current behavior probably relies on whether palloc returns a block with this part of the memory correctly initialized.

Simply zeroing the stack items during creation should fix the problem, as the code seems to rely on that state (it only sets is_exception_handler to true), and should also avoid similar problems in the future:

diff --git a/src/stmtwalk.c b/src/stmtwalk.c
index 40bfe1e..ed19c26 100644
--- a/src/stmtwalk.c
+++ b/src/stmtwalk.c
@@ -1480,7 +1480,7 @@ push_stmt_to_stmt_stack(PLpgSQL_checkstate *cstate)
    PLpgSQL_stmt_stack_item *stmt_stack_item;
    PLpgSQL_stmt_stack_item *current = cstate->top_stmt_stack;

-   stmt_stack_item = (PLpgSQL_stmt_stack_item *) palloc(sizeof(PLpgSQL_stmt_stack_item));
+   stmt_stack_item = (PLpgSQL_stmt_stack_item *) palloc0(sizeof(PLpgSQL_stmt_stack_item));
    stmt_stack_item->stmt = stmt;

    switch (PLPGSQL_STMT_TYPES stmt->cmd_type)

This patch reliably fixes the problem for me. @df7cb can you check if that solves the problem too, or should I send a pull request or something else?

df7cb commented

@okbob: The installation instructions are there: https://wiki.postgresql.org/wiki/Apt/FAQ#I_want_to_try_the_beta_version_of_the_next_PostgreSQL_release

@rjuju: The patch works for me on PG15 (and 14). Thanks!

rjuju commented

Thanks a lot for the confirmation @df7cb, PR sent!

df7cb commented

Thanks, PG15 packages work now!

rjuju commented

Great news!

okbob commented
rjuju commented

If you need it before Thursday, and assuming your system is already configured for pgdg packages, you just need to add this line in /etc/apt/sources.list.d/pgdg.list

deb http://apt.postgresql.org/pub/repos/apt/ buster-pgdg main 15

and then apt update.

okbob commented
rjuju commented

It's providing version 2.2.2:

$ apt show postgresql-15-plpgsql-check
Package: postgresql-15-plpgsql-check
Version: 2.2.2-1.pgdg100+1
[...]

So long after the fix has been released.