Re: A bug with ALTER TABLE SET WITHOUT OIDS in CVS HEAD - Mailing list pgsql-hackers
From | KaiGai Kohei |
---|---|
Subject | Re: A bug with ALTER TABLE SET WITHOUT OIDS in CVS HEAD |
Date | |
Msg-id | 492E24A3.7000806@ak.jp.nec.com Whole thread Raw |
In response to | A bug with ALTER TABLE SET WITHOUT OIDS in CVS HEAD (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>) |
Responses |
Re: A bug with ALTER TABLE SET WITHOUT OIDS in CVS HEAD
|
List | pgsql-hackers |
Heikki Linnakangas wrote: > This patch: > >> commit 35ad25ad66fa3999bbc0bb59ca13cef3d750fb07 >> Author: Tom Lane <tgl@sss.pgh.pa.us> >> Date: Sat Jul 26 19:15:35 2008 +0000 >> >> As noted by Andrew Gierth, there's really no need any more to force a junk >> filter to be used when INSERT or SELECT INTO has a plan that returns raw >> disk tuples. The virtual-tuple-slot optimizations that were put in place >> awhile ago mean that ExecInsert has to do ExecMaterializeSlot, and that >> already copies the tuple if it's raw (and does so more efficiently than >> a junk filter, too). So get rid of that logic. This in turn means that >> we can throw away ExecMayReturnRawTuples, which wasn't used for any other >> purpose, and was always a kluge anyway. >> In passing, move a couple of SELECT-INTO-specific fields out of EState >> and into the private state of the SELECT INTO DestReceiver, as was foreseen >> in an old comment there. Also make intorel_receive use ExecMaterializeSlot >> not ExecCopySlotTuple, for consistency with ExecInsert and to possibly save >> a tuple copy step in some cases. >> > > made this test case crash: > > CREATE TABLE xtable (padding char(2000)) WITH OIDS; > INSERT INTO xtable VALUES('1'); > ALTER TABLE xtable SET WITHOUT OIDS; > INSERT INTO xtable (SELECT * FROM xtable); > > with assertion failure: > > TRAP: FailedAssertion("!(!(tup->t_data->t_infomask & 0x0008))", File: > "heapam.c", Line: 1782) http://wiki.postgresql.org/wiki/PostgreSQL_8.4_Open_Items In addition, it can show us another unexpected behavior. * Before patch applied: postgres=# CREATE TABLE t1 (a int, b text) WITH OIDS; CREATE TABLE postgres=# INSERT INTO t1 VALUES(1,'aaa'), (2,'bbb'), (3,'ccc'); INSERT 0 3 postgres=# SELECT oid,* FROM t1; oid | a | b -------+---+----- 16405| 1 | aaa 16406 | 2 | bbb 16407 | 3 | ccc (3 rows) postgres=# INSERT INTO t1 (SELECT * FROM t1); INSERT 0 3 postgres=# SELECT oid,* FROM t1; oid | a | b -------+---+----- 16405 | 1 | aaa 16406 | 2 | bbb 16407 | 3 | ccc 16405 | 1 | aaa 16406 | 2 | bbb 16407 | 3 | ccc (6rows) The newly insered three tuples preserves its object identifier because the fetched tuples has its valid object identifier which means it does not need to assign a new one. The matter comes from that we cannot guess ahead whether the fetched tuple has object identifier field, or not. Thus, it is necessary to enforce to translate fetched tuples into the current proper rowtype on INSERT, UPDATE or SELECT INTO. If my understanding is correct, the following patch can fix the matters. ---------------------(cut here)--------------------- *** src/backend/executor/execScan.c (revision 1244) --- src/backend/executor/execScan.c (working copy) *************** *** 243,250 **** * If the plan context requires a particular hasoid setting, then that has * to match, too. */ ! if (ExecContextForcesOids(ps, &hasoid) && ! hasoid != tupdesc->tdhasoid) return false; return true; --- 243,249 ---- * If the plan context requires a particular hasoid setting, then that has * to match, too. */ ! if (ExecContextForcesOids(ps, &hasoid)) return false; return true; ---------------------(cut here)--------------------- * After the patch applied: postgres=# CREATE TABLE t1 (a int, b text) WITH OIDS; CREATE TABLE postgres=# INSERT INTO t1 VALUES (1,'aaa'), (2,'bbb'),(3,'ccc'); INSERT 0 3 postgres=# SELECT oid,* FROM t1; oid | a | b -------+---+----- 16435 | 1 | aaa 16436| 2 | bbb 16437 | 3 | ccc (3 rows) postgres=# ALTER TABLE t1 SET WITHOUT OIDS; ALTER TABLE postgres=# INSERT INTO t1 (SELECT * FROM t1); INSERT 0 3 postgres=#SELECT * FROM t1; a | b ---+----- 1 | aaa 2 | bbb 3 | ccc 1 | aaa 2 | bbb 3 | ccc (6 rows) * After patch applied: postgres=# CREATE TABLE t1 (a int, b text) WITH OIDS; CREATE TABLE postgres=# INSERT INTO t1 VALUES(1,'aaa'), (2,'bbb'), (3,'ccc'); INSERT 0 3 postgres=# SELECT oid,* FROM t1; oid | a | b -------+---+----- 16420| 1 | aaa 16421 | 2 | bbb 16422 | 3 | ccc (3 rows) postgres=# INSERT INTO t1 (SELECT * FROM t1); INSERT 0 3 postgres=# SELECT oid,* FROM t1; oid | a | b -------+---+----- 16420 | 1 | aaa 16421 | 2 | bbb 16422 | 3 | ccc 16423 | 1 | aaa 16424 | 2 | bbb 16425 | 3 | ccc (6rows) Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>
pgsql-hackers by date: