Pl/pgsql functions causing crashes in 8.2.2
Following an upgrade to 8.2.2, many of my plpgsql functions started to cause
server process crashes.
I make use of a custom data-type "uniqueidentifier", available here:
http://gborg.postgresql.org/project/uniqueidentifier
ftp://gborg.postgresql.org/pub/uniqueidentifier/stable/uniqueidentifier-0.2.
tar.gz
This type has given me the same kind of process crash once before, but that
was related to NULL values in a foreign-key referenced field (unresolved to
this day, but behavior is allowed for all builtin types).
I see the crash in plpgsql functions that return an aggregate type which
contain a uniqueidentifier (including triggers which have uniqueidentifiers
in the NEW). Here is a test case I was able to create:
<< TEST SETUP >>
CREATE SCHEMA test;
CREATE TYPE test.guid_plus AS (
id public.uniqueidentifier,
num integer
);
CREATE TABLE test.guid_table (
id public.uniqueidentifier,
num integer
);
INSERT INTO test.guid_table (id,num) VALUES (newid(),1);
INSERT INTO test.guid_table (id,num) VALUES (newid(),2);
INSERT INTO test.guid_table (id,num) VALUES (newid(),3);
INSERT INTO test.guid_table (id,num) VALUES (newid(),4);
CREATE OR REPLACE FUNCTION test.break_guid (idlower integer, idupper
integer) RETURNS SETOF test.guid_plus AS
$$
DECLARE
x RECORD;
gplus_ret test.guid_plus;
BEGIN
FOR x IN SELECT id,num FROM test.guid_table WHERE id > idlower AND id <
idupper LOOP
gplus_ret :=
(x.id::uniqueidentifier,x.num::integer)::test.guid_plus;
-- I usually do the following: (but tried above with same result)
-- gplus_ret := (x.id,x.num);
RETURN NEXT gplus_ret;
END LOOP;
RETURN;
END;
$$ LANGUAGE plpgsql;
<< CAUSE THE CRASH >>
SELECT * FROM test.break_guid(0,5);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
<< FROM THE LOGS, SET AT DEBUG5 >>
2007-02-05 22:31:07 PST [31363]: [45-1] DEBUG: StartTransactionCommand
2007-02-05 22:31:07 PST [31363]: [46-1] DEBUG: StartTransaction
2007-02-05 22:31:07 PST [31363]: [47-1] DEBUG: name: unnamed; blockState:
DEFAULT; state: INPROGR, xid/subid/cid: 7198/1/0, nestlvl: 1, children: <>
2007-02-05 22:31:07 PST [31278]: [775-1] DEBUG: reaping dead processes
2007-02-05 22:31:07 PST [31278]: [776-1] DEBUG: server process (PID 31363)
was terminated BY signal 11
2007-02-05 22:31:07 PST [31278]: [777-1] LOG: server process (PID 31363) was
terminated BY signal 11
2007-02-05 22:31:07 PST [31278]: [778-1] LOG: terminating any other active
server processes
2007-02-05 22:31:07 PST [31278]: [779-1] DEBUG: sending SIGQUIT TO process
31361
2007-02-05 22:31:07 PST [31278]: [780-1] DEBUG: reaping dead processes
2007-02-05 22:31:07 PST [31278]: [781-1] LOG: ALL server processes
terminated; reinitializing
2007-02-05 22:31:07 PST [31278]: [782-1] DEBUG: shmem_exit(0)
2007-02-05 22:31:07 PST [31278]: [783-1] DEBUG: invoking
IpcMemoryCreate(size=537141248)
2007-02-05 22:31:07 PST [31364]: [1-1] LOG: DATABASE system was interrupted
at 2007-02-05 22:30:35 PST
2007-02-05 22:31:07 PST [31364]: [2-1] LOG: checkpoint record IS at
0/286D97FC
2007-02-05 22:31:07 PST [31364]: [3-1] LOG: redo record IS at 0/286D97FC;
undo record IS at 0/0; shutdown TRUE
2007-02-05 22:31:07 PST [31364]: [4-1] LOG: next transaction ID: 0/7192;
next OID: 155654
2007-02-05 22:31:07 PST [31364]: [5-1] LOG: next MultiXactId: 1; next
MultiXactOffset: 0
2007-02-05 22:31:07 PST [31364]: [6-1] LOG: DATABASE system was NOT properly
shut down; automatic recovery IN progress
2007-02-05 22:31:07 PST [31365]: [1-1] FATAL: the DATABASE system IS
starting up
2007-02-05 22:31:07 PST [31365]: [2-1] DEBUG: proc_exit(1)
2007-02-05 22:31:07 PST [31365]: [3-1] DEBUG: shmem_exit(1)
2007-02-05 22:31:07 PST [31365]: [4-1] DEBUG: exit(1)
2007-02-05 22:31:07 PST [31278]: [784-1] DEBUG: forked new backend,
pid=31365 socket=8
2007-02-05 22:31:07 PST [31278]: [785-1] DEBUG: reaping dead processes
2007-02-05 22:31:07 PST [31278]: [786-1] DEBUG: server process (PID 31365)
exited WITH exit code 1
2007-02-05 22:31:07 PST [31364]: [7-1] LOG: record WITH zero length at
0/286D9844
2007-02-05 22:31:07 PST [31364]: [8-1] LOG: redo IS NOT required
2007-02-05 22:31:07 PST [31364]: [9-1] LOG: DATABASE system IS ready
2007-02-05 22:31:07 PST [31364]: [10-1] DEBUG: transaction ID wrap LIMIT IS
2147484171, limited BY DATABASE "postgres"
2007-02-05 22:31:07 PST [31364]: [11-1] DEBUG: proc_exit(0)
2007-02-05 22:31:07 PST [31364]: [12-1] DEBUG: shmem_exit(0)
2007-02-05 22:31:07 PST [31364]: [13-1] DEBUG: exit(0)
2007-02-05 22:31:07 PST [31278]: [787-1] DEBUG: reaping dead processes
2007-02-05 22:31:07 PST [31368]: [1-1] DEBUG: proc_exit(0)
2007-02-05 22:31:07 PST [31368]: [2-1] DEBUG: shmem_exit(0)
2007-02-05 22:31:07 PST [31368]: [3-1] DEBUG: exit(0)
2007-02-05 22:31:07 PST [31278]: [788-1] DEBUG: reaping dead processes
The data in test.guid_table looks fine and I am able to SELECT it without
any problems. All my problems seem related to plpgsql functions and the
uniqueidentifier type, but I'm not able to diagnose any further (and could
be wrong).
Any help is greatly appreciated. This is my first post to the mailing lists
but I have been a daily lurker for quite some time now :)
Jonathan Gray
jon5pg@streamy.com
Reading the post again I caught a typo in my query. I had been playing
with variations of this test to try and get it working, but I have had no
success with any combination as long as it returns this kind of type.
I was comparing integers to uniqueidentiers, which actually works, but is
unrelated to the issue.
Should be:
CREATE OR REPLACE FUNCTION test.break_guid (numlower integer, numupper
integer) RETURNS SETOF test.guid_plus AS
$$
DECLARE
x RECORD;
gplus_ret test.guid_plus;
BEGIN
FOR x IN SELECT id,num FROM test.guid_table WHERE num > numlower AND
num
< numupper LOOP
gplus_ret :=
(x.id::uniqueidentifier,x.num::integer)::test.guid_plus;
-- I usually do the following: (but tried above with same result)
-- gplus_ret := (x.id,x.num);
RETURN NEXT gplus_ret;
END LOOP;
RETURN;
END;
$$ LANGUAGE plpgsql;
Jonathan Gray
jon5pg@streamy.com
jon5pg@streamy.com wrote:
Reading the post again I caught a typo in my query. I had been playing
with variations of this test to try and get it working, but I have had no
success with any combination as long as it returns this kind of type.I was comparing integers to uniqueidentiers, which actually works, but is
unrelated to the issue.
Does it still do it if you just return a single uniqueidentifier?
1. RETURN newid()
2. SELECT INTO r newid(); RETURN r;
3. SELECT id INTO r ...query... LIMIT 1; RETURN r;
If all of these fail, then presumably it's allocating memory incorrectly
but only shows up in a function's context.
--
Richard Huxton
Archonet Ltd
On 2/6/07, Jonathan Gray <jon5pg@streamy.com> wrote:
Following an upgrade to 8.2.2, many of my plpgsql functions started to cause
server process crashes.I make use of a custom data-type "uniqueidentifier", available here:
http://gborg.postgresql.org/project/uniqueidentifier
ftp://gborg.postgresql.org/pub/uniqueidentifier/stable/uniqueidentifier-0.2.
tar.gzThis type has given me the same kind of process crash once before, but that
was related to NULL values in a foreign-key referenced field (unresolved to
this day, but behavior is allowed for all builtin types).
Indeed, the code can crash on NULL values as the NULL checks
are missing or wrong in the functions. Actually all the various
functions except newid() should be declared STRICT IMMUTABLE
thus immidiately avoiding problems with NULLs.
Could you reproduce the crash with this change? I'll try
to play with this myself too.
--
marko
On 2/6/07, Marko Kreen <markokr@gmail.com> wrote:
Indeed, the code can crash on NULL values as the NULL checks
are missing or wrong in the functions. Actually all the various
functions except newid() should be declared STRICT IMMUTABLE
thus immidiately avoiding problems with NULLs.Could you reproduce the crash with this change? I'll try
to play with this myself too.
STRICT IMMUTABLE fixed the crash for me so seems it was bug
in the module. Although it did not happen in 8.2.1 so seems
some change in 8.2.2 made it trigger.
Attached is a patch for uniqueindent-0.2 that removes the
buggy checks and makes functions STRICT IMMUTABLE.
--
marko
Attachments:
null.fix.diffapplication/octet-stream; name=null.fix.diffDownload
diff -ur uniqueidentifier-0.2/uniqueidentifier.c uniqueidentifier/uniqueidentifier.c
--- uniqueidentifier-0.2/uniqueidentifier.c 2003-12-01 22:22:35.000000000 +0200
+++ uniqueidentifier/uniqueidentifier.c 2007-02-06 12:22:49.000000000 +0200
@@ -23,6 +23,9 @@
#include "fmgr.h"
#include "access/hash.h"
+#ifdef PG_MODULE_MAGIC
+PG_MODULE_MAGIC;
+#endif
/*
* Here is the internal storage format for uniqueidentifiers.
@@ -97,9 +100,6 @@
char *result;
uniqueidentifier *in = (uniqueidentifier *) PG_GETARG_POINTER(0);
- if (in == NULL)
- PG_RETURN_CSTRING (NULL);
-
result = (char *) palloc(37);
result[0] = '\0';
@@ -206,13 +206,6 @@
uniqueidentifier *u1 = (uniqueidentifier *) PG_GETARG_POINTER(0);
uniqueidentifier *u2 = (uniqueidentifier *) PG_GETARG_POINTER(1);
- if ((u1 == NULL) || (u2 == NULL)) {
- if ((u1 == NULL) && (u2 == NULL))
- PG_RETURN_BOOL (true);
- else
- PG_RETURN_BOOL (false);
- }
-
if (memcmp(u1->uid, u2->uid, 16) == 0)
PG_RETURN_BOOL (true);
else
@@ -243,12 +236,7 @@
{
uniqueidentifier *u1 = (uniqueidentifier *) PG_GETARG_POINTER(0);
uniqueidentifier *u2 = (uniqueidentifier *) PG_GETARG_POINTER(1);
- if ((u1 == NULL) || (u2 == NULL)) {
- if ((u1 == NULL) && (u2 == NULL))
- PG_RETURN_BOOL (false);
- else
- PG_RETURN_BOOL (true);
- }
+
if (memcmp(u1->uid, u2->uid, 16) == 0)
PG_RETURN_BOOL (false);
else
@@ -319,15 +307,6 @@
uint16 time_hi_and_version2;
uint16 clock_seq2;
- if ((u1 == NULL) || (u2 == NULL)) {
- if ((u1 == NULL) && (u2 == NULL))
- return 0;
- if (u1 == NULL)
- return 1;
- else
- return -1;
- }
-
if(memcmp(u1->uid, u2->uid, 16) == 0)
return 0;
diff -ur uniqueidentifier-0.2/uniqueidentifier.sql.in uniqueidentifier/uniqueidentifier.sql.in
--- uniqueidentifier-0.2/uniqueidentifier.sql.in 2003-12-02 14:45:20.000000000 +0200
+++ uniqueidentifier/uniqueidentifier.sql.in 2007-02-06 12:24:42.000000000 +0200
@@ -12,11 +12,13 @@
CREATE OR REPLACE FUNCTION uniqueidentifier_in(cstring)
returns uniqueidentifier
+ strict immutable
as 'MODULE_PATHNAME'
language 'c';
CREATE OR REPLACE FUNCTION uniqueidentifier_out(uniqueidentifier)
returns cstring
+ strict immutable
as 'MODULE_PATHNAME'
language 'c';
@@ -34,31 +36,37 @@
CREATE OR REPLACE FUNCTION uniqueidentifier_lt(uniqueidentifier, uniqueidentifier)
returns bool
as 'MODULE_PATHNAME'
+ strict immutable
language 'c';
CREATE OR REPLACE FUNCTION uniqueidentifier_le(uniqueidentifier, uniqueidentifier)
returns bool
as 'MODULE_PATHNAME'
+ strict immutable
language 'c';
CREATE OR REPLACE FUNCTION uniqueidentifier_eq(uniqueidentifier, uniqueidentifier)
returns bool
as 'MODULE_PATHNAME'
+ strict immutable
language 'c';
CREATE OR REPLACE FUNCTION uniqueidentifier_ge(uniqueidentifier, uniqueidentifier)
returns bool
as 'MODULE_PATHNAME'
+ strict immutable
language 'c';
CREATE OR REPLACE FUNCTION uniqueidentifier_gt(uniqueidentifier, uniqueidentifier)
returns bool
as 'MODULE_PATHNAME'
+ strict immutable
language 'c';
CREATE OR REPLACE FUNCTION uniqueidentifier_ne(uniqueidentifier, uniqueidentifier)
returns bool
as 'MODULE_PATHNAME'
+ strict immutable
language 'c';
--
@@ -135,7 +143,7 @@
CREATE OR REPLACE FUNCTION text(uniqueidentifier)
returns text
- immutable
+ strict immutable
as 'MODULE_PATHNAME', 'uniqueidentifier_text'
language 'c';
@@ -145,7 +153,7 @@
CREATE OR REPLACE FUNCTION newid(text)
returns uniqueidentifier
- immutable
+ strict immutable
as 'MODULE_PATHNAME', 'uniqueidentifier_from_text'
language 'c';
@@ -171,6 +179,7 @@
CREATE OR REPLACE FUNCTION uniqueidentifier_cmp(uniqueidentifier, uniqueidentifier)
RETURNS int4
+ strict immutable
AS 'MODULE_PATHNAME'
LANGUAGE 'C';
On 2/6/07, Marko Kreen <markokr@gmail.com> wrote:
STRICT IMMUTABLE fixed the crash for me so seems it was bug
in the module. Although it did not happen in 8.2.1 so seems
some change in 8.2.2 made it trigger.
Trigger was following patch:
http://archives.postgresql.org/pgsql-committers/2007-02/msg00016.php
as function test.break_guid() assigns NULLs to gplus_ret.
--
marko
"Marko Kreen" <markokr@gmail.com> writes:
Attached is a patch for uniqueindent-0.2 that removes the
buggy checks and makes functions STRICT IMMUTABLE.
Not sure where you should send that, but it's not here.
regards, tom lane
"Marko Kreen" <markokr@gmail.com> writes:
On 2/6/07, Marko Kreen <markokr@gmail.com> wrote:
STRICT IMMUTABLE fixed the crash for me so seems it was bug
in the module. Although it did not happen in 8.2.1 so seems
some change in 8.2.2 made it trigger.
Trigger was following patch:
http://archives.postgresql.org/pgsql-committers/2007-02/msg00016.php
as function test.break_guid() assigns NULLs to gplus_ret.
So in fact the problem was that the input function was not declared
STRICT and yet failed to handle nulls... which means it was broken
as of 8.2.0, the OP just hadn't tried to throw a null at it except
in the context of plpgsql ...
regards, tom lane
On 2/6/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Marko Kreen" <markokr@gmail.com> writes:
Attached is a patch for uniqueindent-0.2 that removes the
buggy checks and makes functions STRICT IMMUTABLE.Not sure where you should send that, but it's not here.
I did Cc: the maintainer.
--
marko