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?
--
=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive PostgreSQL solutions since 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
isalvherre=# update pk set id = count(id) ;
ERROR: ctid is NULLHmm, 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