Thread: postgres crashing on a seemingly good query

postgres crashing on a seemingly good query

From
Abhijit Menon-Sen
Date:
Summary: I can crash 7.4-CVS and 8.0/HEAD by sending what appears to be
a valid query.

I'm trying to insert a unique entry into this table:
   create table bodyparts (        id          serial primary key,       bytes       integer not null,       lines
integer not null,       hash        text not null,       text        bytea,       data        bytea   );
 
   insert into bodyparts (bytes,lines,hash,text,data)       select $1,$2,$3,$4,$5 where not exists           (select id
frombodyparts where hash=$3)
 
   ($1 = 58, $2 = 3, $3 = "3dd93158c8775c62e7a546ae1abad1da",    $4 = "Appended is one html file and one French
lesson.\r\n\r\nArnt\r\n",   $5 is NULL)
 

I issue a Parse (leaving all the placeholder types unspecified), Bind,
Describe, Execute, and Sync sequence, and the backend segfaults during
the Execute (some whitespace munged to make lines shorter):
 Program received signal SIGSEGV, Segmentation fault. 0x08075071 in ComputeDataSize (tupleDesc=0x83ed778,
values=0x83edab8,    nulls=0x83edae4 "    n~\177\177") at heaptuple.c:55 55 data_length = att_addlength(data_length,
att[i]->attlen,values[i]);
 
 (gdb) info local data_length = 0 i = 0 numberOfAttributes = 5 att = (Form_pg_attribute *) 0x83ed7a4 (gdb) p att[i] $7
=0x83ed7d0 (gdb) p *$7 $8 = {attrelid = 0, attname = {data = "?column?", '\0' <repeats 55 times>,       alignmentDummy
=1819239231}, atttypid = 705, attstattarget = -1,       attlen = -1, attnum = 1, attndims = 0, attcacheoff = -1,
atttypmod= -1, attbyval = 0 '\0', attstorage = 112 'p',       attalign = 105 'i', attnotnull = 0 '\0', atthasdef = 0
'\0',      attisdropped = 0 '\0', attislocal = 1 '\001', attinhcount = 0} (gdb) p values[i] $9 = 58
 

Here's a backtrace:
 (gdb) bt #0  0x08075071 in ComputeDataSize (tupleDesc=0x83ed778, values=0x83edab8,     nulls=0x83edae4 "
n~\177\177")at heaptuple.c:55 #1  0x08076414 in heap_formtuple (tupleDescriptor=0x83ed778, values=0x83edab8,
nulls=0x83edae4"    n~\177\177") at heaptuple.c:622 #2  0x0814e343 in ExecTargetList (targetlist=0x83ed594,
targettype=0x83ed778,    econtext=0x83ed4cc, values=0x83edab8, nulls=0x83edae4 "    n~\177\177",
itemIsDone=0x83edaf8,isDone=0xbfffdad4) at execQual.c:3676 #3  0x0814e3a1 in ExecProject (projInfo=0x83eda8c,
isDone=0xbfffdad4)    at execQual.c:3714 #4  0x0815900b in ExecResult (node=0x83ed440) at nodeResult.c:155 #5
0x08147b9cin ExecProcNode (node=0x83ed440) at execProcnode.c:292 #6  0x0815d659 in SubqueryNext (node=0x83eb878) at
nodeSubqueryscan.c:77#7  0x0814e4d6 in ExecScan (node=0x83eb878, accessMtd=0x815d610 <SubqueryNext>)     at
execScan.c:98#8  0x0815d691 in ExecSubqueryScan (node=0x83eb878) at nodeSubqueryscan.c:102 #9  0x08147c0a in
ExecProcNode(node=0x83eb878) at execProcnode.c:315 #10 0x081460f5 in ExecutePlan (estate=0x83eb284,
planstate=0x83eb878,    operation=CMD_INSERT, numberTuples=0, direction=ForwardScanDirection,     dest=0x8326568) at
execMain.c:1060#11 0x08145245 in ExecutorRun (queryDesc=0x83eac44,     direction=ForwardScanDirection, count=0) at
execMain.c:226#12 0x081e41ed in ProcessQuery (parsetree=0x83df51c, plan=0x83fc408,     params=0x83eaa74,
dest=0x8326568,completionTag=0xbfffde70 "")     at pquery.c:173 #13 0x081e5287 in PortalRunMulti (portal=0x83f2a7c,
dest=0x8326568,    altdest=0x8326568, completionTag=0xbfffde70 "") at pquery.c:1016 #14 0x081e4adf in PortalRun
(portal=0x83f2a7c,count=2147483647,     dest=0x83dad20, altdest=0x83dad20, completionTag=0xbfffde70 "")     at
pquery.c:617#15 0x081e1b86 in exec_execute_message (portal_name=0x83dac14 "",     max_rows=2147483647) at
postgres.c:1673#16 0x081e374d in PostgresMain (argc=4, argv=0x836f7d4,     username=0x836f7a4 "oryx") at
postgres.c:3056#17 0x081ad777 in BackendRun (port=0x837ef80) at postmaster.c:2816 #18 0x081acdc5 in BackendStartup
(port=0x837ef80)at postmaster.c:2452 #19 0x081aaf9a in ServerLoop () at postmaster.c:1199 #20 0x081aa8b3 in
PostmasterMain(argc=3, argv=0x836dd98) at postmaster.c:918 #21 0x0816b0e6 in main (argc=3, argv=0x836dd98) at
main.c:268

I couldn't reproduce this with SQL PREPARE/EXECUTE statements in psql,
so I conjectured that it may have something to do with the parameter
types being unspecified. I added a statement-describe message between
the parse and the bind, however, and the server had correctly inferred
all of the types (23/23/25/17/17, i.e. INT4/INT4/TEXT/BYTEA/BYTEA).

I have a tcpdump capture of the transaction, which is available at
<http://wiw.org/~ams/crash.tcpdump>

The most convenient way to view that is to use Ethereal 0.10.9 or later,
since it includes the dissector that I wrote. Unfortunately, between the
time I contributed the code and the 0.10.9 release, someone introduced a
couple of bugs. In the presence of NULL parameter values, Ethereal may
wrongly report some Bind/Data-row messages as being malformed. :-(

You may either choose to ignore this (the hex dump shows that the packet
is correct), or recompile Ethereal with epan/dissectors/packet-pgsql.c
replaced with <http://www.oryx.com/ams/packet-pgsql.c>.

I can reproduce the problem at will, so if there's any other information
that might be useful in tracking down the problem, just ask.

-- ams


Re: postgres crashing on a seemingly good query

From
Abhijit Menon-Sen
Date:
At 2005-02-19 21:38:56 +0530, ams@oryx.com wrote:
>
> Summary: I can crash 7.4-CVS and 8.0/HEAD by sending what appears to be
> a valid query.

A couple of things I forgot to mention:

1. I turned the logging all the way up and I've uploaded the messages  from the crashing backend to
<http://wiw.org/~ams/crash.log> (Only the messages logged after it received the last query.)
 

2. It isn't obviously something my application is doing wrong, since the  "insert into ... select where not exists
(...)"style of query does  work for me in other places (albeit simpler queries). Furthermore,  if I change the query to
asimple "insert into bodyparts ...", it  works fine, so it's not bogus parameter data or anything that's  causing the
problem.
  (Of course, the server probably shouldn't crash even if it were.)

-- ams


Re: postgres crashing on a seemingly good query

From
Abhijit Menon-Sen
Date:
At 2005-02-19 21:38:56 +0530, ams@oryx.com wrote:
>
> I couldn't reproduce this with SQL PREPARE/EXECUTE statements in psql,
> so I conjectured that it may have something to do with the parameter
> types being unspecified. I added a statement-describe message between
> the parse and the bind, however, and the server had correctly inferred
> all of the types (23/23/25/17/17, i.e. INT4/INT4/TEXT/BYTEA/BYTEA).

Another data point: If I change my code (by means of an unspeakably vile
hack, but never mind that) to specify each parameter types in the parse
message, the server no longer crashes.

Any ideas?

-- ams


Re: postgres crashing on a seemingly good query

From
Abhijit Menon-Sen
Date:
At 2005-02-19 23:18:01 +0530, ams@oryx.com wrote:
>
> If I change my code (by means of an unspeakably vile hack, but never
> mind that) to specify each parameter types in the parse message, the
> server no longer crashes.

In fact, it works if I specify only the two integer types, and leave the
three varlenas unspecified.

-- ams


Re: postgres crashing on a seemingly good query

From
Tom Lane
Date:
Abhijit Menon-Sen <ams@oryx.com> writes:
> Summary: I can crash 7.4-CVS and 8.0/HEAD by sending what appears to be
> a valid query.

Good catch.  I've applied the attached patch (this is against 8.0/CVS
tip but applies with some fuzz to 7.4).
        regards, tom lane

*** src/backend/parser/analyze.c.orig    Fri Dec 31 17:45:54 2004
--- src/backend/parser/analyze.c    Sat Feb 19 13:57:51 2005
***************
*** 506,511 ****
--- 506,513 ----                     List **extras_before, List **extras_after) {     Query       *qry =
makeNode(Query);
+     Query       *selectQuery = NULL;
+     bool        copy_up_hack = false;     List       *sub_rtable;     List       *sub_namespace;     List
*icolumns;
***************
*** 561,567 ****          * be able to see.          */         ParseState *sub_pstate = make_parsestate(pstate);
-         Query       *selectQuery;         RangeTblEntry *rte;         RangeTblRef *rtr; 
--- 563,568 ----
***************
*** 608,614 ****         Assert(rte == rt_fetch(rtr->rtindex, pstate->p_rtable));         pstate->p_joinlist =
lappend(pstate->p_joinlist,rtr); 
 
!         /*          * Generate a targetlist for the INSERT that selects all the          * non-resjunk columns from
thesubquery.  (We need this to be          * separate from the subquery's tlist because we may add columns,
 
--- 609,615 ----         Assert(rte == rt_fetch(rtr->rtindex, pstate->p_rtable));         pstate->p_joinlist =
lappend(pstate->p_joinlist,rtr); 
 
!         /*----------          * Generate a targetlist for the INSERT that selects all the          * non-resjunk
columnsfrom the subquery.  (We need this to be          * separate from the subquery's tlist because we may add
columns,
***************
*** 618,625 ****          * are copied up as-is rather than being referenced as subquery          * outputs.  This is
toensure that when we try to coerce them to          * the target column's datatype, the right things happen (see
 
!          * special cases in coerce_type).  Otherwise, this fails: INSERT
!          * INTO foo SELECT 'bar', ... FROM baz          */         qry->targetList = NIL;         foreach(tl,
selectQuery->targetList)
--- 619,627 ----          * are copied up as-is rather than being referenced as subquery          * outputs.  This is
toensure that when we try to coerce them to          * the target column's datatype, the right things happen (see
 
!          * special cases in coerce_type).  Otherwise, this fails:
!          *        INSERT INTO foo SELECT 'bar', ... FROM baz
!          *----------          */         qry->targetList = NIL;         foreach(tl, selectQuery->targetList)
***************
*** 631,639 ****             if (resnode->resjunk)                 continue;             if (tle->expr &&
!                 (IsA(tle->expr, Const) ||IsA(tle->expr, Param)) &&                 exprType((Node *) tle->expr) ==
UNKNOWNOID)                expr = tle->expr;             else                 expr = (Expr *) makeVar(rtr->rtindex,
                                   resnode->resno,
 
--- 633,644 ----             if (resnode->resjunk)                 continue;             if (tle->expr &&
!                 (IsA(tle->expr, Const) || IsA(tle->expr, Param)) &&                 exprType((Node *) tle->expr) ==
UNKNOWNOID)
+             {                 expr = tle->expr;
+                 copy_up_hack = true;
+             }             else                 expr = (Expr *) makeVar(rtr->rtindex,
      resnode->resno,
 
***************
*** 702,707 ****
--- 707,734 ----         ereport(ERROR,                 (errcode(ERRCODE_SYNTAX_ERROR),              errmsg("INSERT has
moretarget columns than expressions")));
 
+ 
+     /*
+      * If we copied up any unknown Params (see HACK above) then their
+      * resolved types need to be propagated into the Resdom nodes of
+      * the sub-INSERT's tlist.  One hack begets another :-(
+      */
+     if (copy_up_hack)
+     {
+         foreach(tl, selectQuery->targetList)
+         {
+             TargetEntry *tle = (TargetEntry *) lfirst(tl);
+             Resdom       *resnode = tle->resdom;
+ 
+             if (resnode->resjunk)
+                 continue;
+             if (resnode->restype == UNKNOWNOID)
+             {
+                 resnode->restype = exprType((Node *) tle->expr);
+                 resnode->restypmod = exprTypmod((Node *) tle->expr);
+             }
+         }
+     }      /* done building the range table and jointree */     qry->rtable = pstate->p_rtable;


Re: postgres crashing on a seemingly good query

From
Neil Conway
Date:
On Sat, 2005-02-19 at 14:35 -0500, Tom Lane wrote:
> Good catch.  I've applied the attached patch (this is against 8.0/CVS
> tip but applies with some fuzz to 7.4).

Is there a way to repro this via SQL? (It would be nice to have a
regression test...)

-Neil




Re: postgres crashing on a seemingly good query

From
Tom Lane
Date:
Neil Conway <neilc@samurai.com> writes:
> On Sat, 2005-02-19 at 14:35 -0500, Tom Lane wrote:
>> Good catch.  I've applied the attached patch (this is against 8.0/CVS
>> tip but applies with some fuzz to 7.4).

> Is there a way to repro this via SQL? (It would be nice to have a
> regression test...)

No, because there's no way to specify a parameter of unknown type at
the SQL level.

The V3 protocol has a whole lot of behavior that cannot be tested by
psql scripts.  Maybe we ought to think about adding some other kind
of test mechanism for it.
        regards, tom lane