snprintf assert is broken by plpgsql #option dump
Hi
Today I had broken plpgsql_check tests aganst PostgreSQL 12. After command
create or replace function ml_trg()
returns trigger as $$
#option dump
declare
begin
if TG_OP = 'INSERT' then
if NEW.status_from IS NULL then
begin
-- performance issue only
select status into NEW.status_from
from pa
where pa_id = NEW.pa_id;
-- nonexist target value
select status into NEW.status_from_xxx
from pa
where pa_id = NEW.pa_id;
exception
when DATA_EXCEPTION then
new.status_from := 'DE';
end;
end if;
end if;
if TG_OP = 'DELETE' then return OLD; else return NEW; end if;
exception
when OTHERS then
NULL;
if TG_OP = 'DELETE' then return OLD; else return NEW; end if;
end;
$$ language plpgsql;
I got backtrace:
Program received signal SIGABRT, Aborted.
0x00007f2c140e653f in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x00007f2c140e653f in raise () from /lib64/libc.so.6
#1 0x00007f2c140d0895 in abort () from /lib64/libc.so.6
#2 0x00000000008b7903 in ExceptionalCondition
(conditionName=conditionName@entry=0xb00d3b "!(strvalue != ((void *)0))",
errorType=errorType@entry=0x906034 "FailedAssertion",
fileName=fileName@entry=0xb00d30 "snprintf.c",
lineNumber=lineNumber@entry=674) at assert.c:54
#3 0x00000000008ff368 in dopr (target=target@entry=0x7fff4ff6aad0,
format=0x7f2bfe4b0de3 " fields",
format@entry=0x7f2bfe4b0dda "ROW %-16s fields",
args=args@entry=0x7fff4ff6ab18)
at snprintf.c:674
#4 0x00000000008ff6b7 in pg_vfprintf (stream=<optimized out>,
fmt=fmt@entry=0x7f2bfe4b0dda
"ROW %-16s fields",
args=args@entry=0x7fff4ff6ab18) at snprintf.c:261
#5 0x00000000008ff7ff in pg_printf (fmt=fmt@entry=0x7f2bfe4b0dda "ROW
%-16s fields") at snprintf.c:286
#6 0x00007f2bfe4a7c67 in plpgsql_dumptree (func=func@entry=0x26811e8) at
pl_funcs.c:1676
#7 0x00007f2bfe49a136 in do_compile (fcinfo=fcinfo@entry=0x7fff4ff6afa0,
procTup=procTup@entry=0x7f2bfe4ba100, function=0x26811e8,
function@entry=0x0, hashkey=hashkey@entry=0x7fff4ff6ad20,
forValidator=forValidator@entry=true) at pl_comp.c:794
#8 0x00007f2bfe49a27a in plpgsql_compile (fcinfo=fcinfo@entry=0x7fff4ff6afa0,
forValidator=forValidator@entry=true) at pl_comp.c:224
#9 0x00007f2bfe497126 in plpgsql_validator (fcinfo=<optimized out>) at
pl_handler.c:504
#10 0x00000000008c03df in OidFunctionCall1Coll
(functionId=functionId@entry=13392,
collation=collation@entry=0, arg1=arg1@entry=40960)
at fmgr.c:1418
#11 0x0000000000563444 in ProcedureCreate (procedureName=<optimized out>,
procNamespace=procNamespace@entry=2200,
replace=<optimized out>, returnsSet=<optimized out>,
returnType=<optimized out>, proowner=16384, languageObjectId=13393,
languageValidator=13392,
prosrc=0x264feb8 "\n#option
There are new assert
Assert(strvalue != NULL);
probably all refname usage inside plpgsql dump functions has problem with
it.
I found two parts
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index b93f866223..d97997c001 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -1519,7 +1519,7 @@ dump_execsql(PLpgSQL_stmt_execsql *stmt)
dump_ind();
printf(" INTO%s target = %d %s\n",
stmt->strict ? " STRICT" : "",
- stmt->target->dno, stmt->target->refname);
+ stmt->target->dno, stmt->target->refname ?
stmt->target->refname : "null");
}
dump_indent -= 2;
}
@@ -1673,7 +1673,7 @@ plpgsql_dumptree(PLpgSQL_function *func)
PLpgSQL_row *row = (PLpgSQL_row *) d;
int i;- printf("ROW %-16s fields", row->refname);
+ printf("ROW %-16s fields", row->refname ? row->refname
: "null");
for (i = 0; i < row->nfields; i++)
{
printf(" %s=var %d", row->fieldnames[i],Regards
Pavel
Pavel Stehule <pavel.stehule@gmail.com> writes:
There are new assert
Assert(strvalue != NULL);
probably all refname usage inside plpgsql dump functions has problem with
it.
This isn't so much a "new assert" as a modeling of the fact that some
printf implementations dump core on a null string pointer, and have done
so since the dawn of time.
Now that we only use snprintf.c in HEAD, it'd be possible to consider
modeling glibc's behavior instead, ie instead of the Assert do
if (strvalue == NULL)
strvalue = "(null)";
I do not think this would be a good idea though, at least not till all
release branches that *don't* always use snprintf.c are out of support.
Every assert failure that we find here is a live bug in the back
branches, even if it happens not to occur on $your-favorite-platform.
Even once that window elapses, I'd not be especially on board with
snprintf.c papering over such cases. They're bugs really.
I found two parts
Thanks for the report, will push something.
regards, tom lane
I wrote:
Pavel Stehule <pavel.stehule@gmail.com> writes:
I found two parts
Thanks for the report, will push something.
On closer look, I'm not sure that these are the only places that are
assuming that any PLpgSQL_variable struct has a refname. What seems
like a safer answer is to make sure they all do, more or less as
attached.
regards, tom lane
Attachments:
assign-valid-refname-for-plpgsql-rows-1.patchtext/x-diff; charset=us-ascii; name=assign-valid-refname-for-plpgsql-rows-1.patchDownload+13-7
čt 4. 10. 2018 v 23:57 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
I wrote:
Pavel Stehule <pavel.stehule@gmail.com> writes:
I found two parts
Thanks for the report, will push something.
On closer look, I'm not sure that these are the only places that are
assuming that any PLpgSQL_variable struct has a refname. What seems
like a safer answer is to make sure they all do, more or less as
attached.
+1
Pavel
Show quoted text
regards, tom lane