Thread: UPDATE crash in HEAD and 8.1
A bug reported by Josh Drake, crashes 8.1 and CVS HEAD: Test case is: create table pk (id bigserial primary key); insert into pk values (DEFAULT); insert into pk values (DEFAULT); insert into pk values (DEFAULT); update pk set id = max(id) + 2; -- SEGV simple eh? :-) The backtrace on HEAD looks like this: (gdb) bt #0 slot_getattr (slot=0x0, attnum=-1, isnull=0x83fc3f9 "\177~\177\177\177\177\177¬\005A\b@") at /pgsql/source/00orig/src/backend/access/common/heaptuple.c:1288 #1 0x08168ae1 in ExecProject (projInfo=0x83fc40c, isDone=0xafecda2c) at /pgsql/source/00orig/src/backend/executor/execQual.c:3847 #2 0x08176c9d in ExecResult (node=0x83fbce0) at /pgsql/source/00orig/src/backend/executor/nodeResult.c:157 #3 0x081647e5 in ExecProcNode (node=0x83fbce0) at /pgsql/source/00orig/src/backend/executor/execProcnode.c:329 #4 0x0816315b in ExecutorRun (queryDesc=0x8404698, direction=ForwardScanDirection, count=0) at /pgsql/source/00orig/src/backend/executor/execMain.c:1139 #5 0x081f8298 in ProcessQuery (parsetree=<value optimized out>, plan=0x8412670, params=0x0, dest=0x8412754, completionTag=0xafecdcb8"") at /pgsql/source/00orig/src/backend/tcop/pquery.c:174 #6 0x081f94c2 in PortalRun (portal=0x84024bc, count=2147483647, dest=0x8412754, altdest=0x8412754, completionTag=0xafecdcb8"") at /pgsql/source/00orig/src/backend/tcop/pquery.c:1079 #7 0x081f484f in exec_simple_query ( query_string=0x83f0ffc "update pk set id = max(id) + 2;") at /pgsql/source/00orig/src/backend/tcop/postgres.c:1025 NULL slot!? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> update pk set id = max(id) + 2;
I'm fairly sure this query is illegal per spec. There are ancient
discussions in the archives about whether aggregates in an UPDATE target
list can have a consistent interpretation or not. We never found one,
but never got around to disallowing it either. Maybe it's time. If you
try it with something like sum() you don't get a crash, but you do get
rather bizarre behavior.
Having said that, this may well expose a bug in the MAX-optimization
code that has consequences for more useful queries. I'll take a look
later today if no one beats me to it.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > >> update pk set id = max(id) + 2; >> > > I'm fairly sure this query is illegal per spec. There are ancient > discussions in the archives about whether aggregates in an UPDATE target > list can have a consistent interpretation or not. We never found one, > but never got around to disallowing it either. Maybe it's time. If you > try it with something like sum() you don't get a crash, but you do get > rather bizarre behavior. > > Having said that, this may well expose a bug in the MAX-optimization > code that has consequences for more useful queries. I'll take a look > later today if no one beats me to it. > > If you try the query on 8.0 and before you don't get a crash either, but the result is unexpected. Try this version: create table pk (id bigserial primary key, orig bigint); insert into pk (id) values (DEFAULT); insert into pk (id) values (DEFAULT); insert into pk (id) values (DEFAULT); update pk set orig = id; select * from pk; update pk set id = max(id) + 2; select * from pk; One could almost argue that crashing would be better ;-) cheers andrew
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > update pk set id = max(id) + 2; > > I'm fairly sure this query is illegal per spec. There are ancient > discussions in the archives about whether aggregates in an UPDATE target > list can have a consistent interpretation or not. We never found one, > but never got around to disallowing it either. Maybe it's time. If you > try it with something like sum() you don't get a crash, but you do get > rather bizarre behavior. Yeah, I agree we should disallow it. For the curious, the bizarre behavior is alvherre=# update pk set id = count(id) ; ERROR: ctid is NULL alvherre=# update pk set id = sum(id) ; ERROR: ctid is NULL Clearly not a very useful error message. > Having said that, this may well expose a bug in the MAX-optimization > code that has consequences for more useful queries. I'll take a look > later today if no one beats me to it. I refrain -- tried following it but I don't know that code at all. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: >> update pk set id = max(id) + 2; > > I'm fairly sure this query is illegal per spec. There are ancient > discussions in the archives about whether aggregates in an UPDATE target > list can have a consistent interpretation or not. We never found one, > but never got around to disallowing it either. Maybe it's time. If you > try it with something like sum() you don't get a crash, but you do get > rather bizarre behavior. On 8.x (7.4 and 7.3 as well) it will update "1" row :). On 8.1 and HEAD it crashes. This has been verified on 32bit, 64bit x86 and PPC. > Having said that, this may well expose a bug in the MAX-optimization > code that has consequences for more useful queries. I'll take a look > later today if no one beats me to it. Sincerely, Joshua D. Drake > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Have you searched our list archives? > > http://archives.postgresql.org > -- === The PostgreSQL Company: Command Prompt, Inc. === Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240 Providing the most comprehensive PostgreSQL solutionssince 1997 http://www.commandprompt.com/
Alvaro Herrera <alvherre@commandprompt.com> writes: > Tom Lane wrote: >> I'm fairly sure this query is illegal per spec. There are ancient >> discussions in the archives about whether aggregates in an UPDATE target >> list can have a consistent interpretation or not. We never found one, >> but never got around to disallowing it either. Maybe it's time. If you >> try it with something like sum() you don't get a crash, but you do get >> rather bizarre behavior. > Yeah, I agree we should disallow it. For the curious, the bizarre behavior > is > alvherre=# update pk set id = count(id) ; > ERROR: ctid is NULL Hmm, what version are you testing? What I see is that it updates a single one of the table rows :-( I found the previous discussion (or one such, anyway): http://archives.postgresql.org/pgsql-bugs/2000-07/msg00046.php That message mentions "ctid is NULL" in the context of a join update, but for the single-table case, all the versions I've tried seem to do the other thing. It's pretty broken either way of course ... regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Tom Lane wrote: > >> I'm fairly sure this query is illegal per spec. There are ancient > >> discussions in the archives about whether aggregates in an UPDATE target > >> list can have a consistent interpretation or not. We never found one, > >> but never got around to disallowing it either. Maybe it's time. If you > >> try it with something like sum() you don't get a crash, but you do get > >> rather bizarre behavior. > > > Yeah, I agree we should disallow it. For the curious, the bizarre behavior > > is > > > alvherre=# update pk set id = count(id) ; > > ERROR: ctid is NULL > > Hmm, what version are you testing? What I see is that it updates a > single one of the table rows :-( The trick seems to be that the table must be empty. I'm doing this in 8.1.3. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> Having said that, this may well expose a bug in the MAX-optimization
>> code that has consequences for more useful queries. I'll take a look
>> later today if no one beats me to it.
> I refrain -- tried following it but I don't know that code at all.
I concluded that neither the planner nor the executor is doing anything
particularly wrong here. The crash comes because adding the implicit
CTID reference results in a variable with no scan referent at all, if
all the table scans have been pushed down into InitPlans by the MIN/MAX
index optimization. But there isn't any legal query variant that could
do the same thing, so I see no need to install more defenses than
disallowing the query. Which I've now done.
regards, tom lane