Attached is a patch for temp tables that implements Tom's requests.
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> >> But, when a temp rel is dropped it seems that heap_drop_with_catalog also
> >> drops the indexes, so the error occurs when index_drop is called (at least
> >> I think this is the case). Certainly commenting out the last two lines
> >> *seems* to work.
>
> > Not sure why I introduced that bug in 1.18. Your suggestion was 100%
> > correct. I have applied the following patch.
>
> Actually, I don't think this is the true explanation. index_drop'ing
> the temp indexes may not be *necessary*, but it shouldn't *hurt* either.
>
> Now that I think about it, the reason for the failure is probably that
> there's no CommandCounterIncrement in this loop. Therefore, when we
> index_drop an index, the resulting system-table updates are *not seen*
> by heap_drop_with_catalog when it comes time to drop the owning table,
> and so it tries to drop the index again.
>
> Your solution of not doing index_drop at all is sufficient for the
> table-and-index case, but I bet it is not sufficient for more complex
> cases like RI checks between temp relations. I'd recommend doing
> CommandCounterIncrement after each temp item is dropped, instead.
>
> There is another potential bug here: remove_all_temp_relations() is
> failing to consider the possibility that removing one list entry may
> cause other ones (eg, indexes) to go away. It's holding onto a "next"
> pointer to a list entry that may not be there by the time control comes
> back from heap_drop_with_catalog. This is probably OK for tables and
> indexes because the indexes will always be added after their table and
> thus appear earlier in the list, but again it seems like trouble just
> waiting to happen. I would recommend logic along the lines of
>
> while (temp_rels != NIL)
> {
> get first entry in temp_rels,
> and either heap_drop or index_drop it as appropriate;
> CommandCounterIncrement();
> }
>
> This relies on the drop to come back and remove the temp_rels entry
> (and, possibly, other entries); so it's not an infinite loop even
> though it looks like one.
>
> regards, tom lane
>
--
Bruce Momjian | http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
? config.log
? config.cache
? config.status
? GNUmakefile
? src/Makefile.custom
? src/GNUmakefile
? src/Makefile.global
? src/log
? src/crtags
? src/backend/postgres
? src/backend/catalog/global.bki
? src/backend/catalog/global.description
? src/backend/catalog/template1.bki
? src/backend/catalog/template1.description
? src/backend/port/Makefile
? src/bin/x
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_config/pg_config
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_restore
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/pgaccess/pgaccess
? src/bin/pgtclsh/Makefile.tkdefs
? src/bin/pgtclsh/Makefile.tcldefs
? src/bin/pgtclsh/pgtclsh
? src/bin/pgtclsh/pgtksh
? src/bin/psql/psql
? src/bin/scripts/createlang
? src/include/config.h
? src/include/stamp-h
? src/interfaces/ecpg/lib/libecpg.so.3.2.0
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgeasy/libpgeasy.so.2.1
? src/interfaces/libpgtcl/libpgtcl.so.2.1
? src/interfaces/libpq/libpq.so.2.1
? src/interfaces/perl5/blib
? src/interfaces/perl5/Makefile
? src/interfaces/perl5/pm_to_blib
? src/interfaces/perl5/Pg.c
? src/interfaces/perl5/Pg.bs
? src/pl/plpgsql/src/libplpgsql.so.1.0
? src/pl/tcl/Makefile.tcldefs
? src/test/regress/pg_regress
? src/test/regress/regress.out
? src/test/regress/results
? src/test/regress/regression.diffs
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/constraints.out
? src/test/regress/sql/copy.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/constraints.sql
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.71
diff -c -r1.71 xact.c
*** src/backend/access/transam/xact.c 2000/09/27 10:41:55 1.71
--- src/backend/access/transam/xact.c 2000/10/11 21:22:55
***************
*** 1119,1125 ****
AtEOXact_portals();
RecordTransactionAbort();
RelationPurgeLocalRelation(false);
! invalidate_temp_relations();
AtEOXact_SPI();
AtEOXact_nbtree();
AtAbort_Cache();
--- 1119,1125 ----
AtEOXact_portals();
RecordTransactionAbort();
RelationPurgeLocalRelation(false);
! remove_temp_rel_in_myxid();
AtEOXact_SPI();
AtEOXact_nbtree();
AtAbort_Cache();
Index: src/backend/catalog/heap.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.147
diff -c -r1.147 heap.c
*** src/backend/catalog/heap.c 2000/10/05 19:48:21 1.147
--- src/backend/catalog/heap.c 2000/10/11 21:22:59
***************
*** 131,141 ****
MaxCommandIdAttributeNumber, 0, -1, -1, '\001', 'p', '\0', 'i', '\0', '\0'
};
! /*
We decide to call this attribute "tableoid" rather than say
"classoid" on the basis that in the future there may be more than one
table of a particular class/type. In any case table is still the word
! used in SQL.
*/
static FormData_pg_attribute a7 = {
0xffffffff, {"tableoid"}, OIDOID, 0, sizeof(Oid),
--- 131,141 ----
MaxCommandIdAttributeNumber, 0, -1, -1, '\001', 'p', '\0', 'i', '\0', '\0'
};
! /*
We decide to call this attribute "tableoid" rather than say
"classoid" on the basis that in the future there may be more than one
table of a particular class/type. In any case table is still the word
! used in SQL.
*/
static FormData_pg_attribute a7 = {
0xffffffff, {"tableoid"}, OIDOID, 0, sizeof(Oid),
***************
*** 1489,1495 ****
RelationForgetRelation(rid);
if (istemp)
! remove_temp_relation(rid);
if (has_toasttable)
{
--- 1489,1495 ----
RelationForgetRelation(rid);
if (istemp)
! remove_temp_rel_by_relid(rid);
if (has_toasttable)
{
Index: src/backend/catalog/index.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.127
diff -c -r1.127 index.c
*** src/backend/catalog/index.c 2000/10/05 19:48:21 1.127
--- src/backend/catalog/index.c 2000/10/11 21:23:00
***************
*** 1145,1151 ****
RelationForgetRelation(indexId);
/* does something only if it is a temp index */
! remove_temp_relation(indexId);
}
/* ----------------------------------------------------------------
--- 1145,1151 ----
RelationForgetRelation(indexId);
/* does something only if it is a temp index */
! remove_temp_rel_by_relid(indexId);
}
/* ----------------------------------------------------------------
***************
*** 1374,1380 ****
if (!LockClassinfoForUpdate(relid, &tuple, &buffer, confirmCommitted))
elog(ERROR, "IndexesAreActive couldn't lock %u", relid);
if (((Form_pg_class) GETSTRUCT(&tuple))->relkind != RELKIND_RELATION &&
! ((Form_pg_class) GETSTRUCT(&tuple))->relkind != RELKIND_TOASTVALUE)
elog(ERROR, "relation %u isn't an indexable relation", relid);
isactive = ((Form_pg_class) GETSTRUCT(&tuple))->relhasindex;
ReleaseBuffer(buffer);
--- 1374,1380 ----
if (!LockClassinfoForUpdate(relid, &tuple, &buffer, confirmCommitted))
elog(ERROR, "IndexesAreActive couldn't lock %u", relid);
if (((Form_pg_class) GETSTRUCT(&tuple))->relkind != RELKIND_RELATION &&
! ((Form_pg_class) GETSTRUCT(&tuple))->relkind != RELKIND_TOASTVALUE)
elog(ERROR, "relation %u isn't an indexable relation", relid);
isactive = ((Form_pg_class) GETSTRUCT(&tuple))->relhasindex;
ReleaseBuffer(buffer);
Index: src/backend/utils/cache/temprel.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/utils/cache/temprel.c,v
retrieving revision 1.27
diff -c -r1.27 temprel.c
*** src/backend/utils/cache/temprel.c 2000/07/12 18:04:45 1.27
--- src/backend/utils/cache/temprel.c 2000/10/11 21:23:01
***************
*** 91,125 ****
void
remove_all_temp_relations(void)
{
- List *l,
- *next;
-
- if (temp_rels == NIL)
- return;
-
AbortOutOfAnyTransaction();
StartTransactionCommand();
! l = temp_rels;
! while (l != NIL)
{
! TempTable *temp_rel = (TempTable *) lfirst(l);
! next = lnext(l); /* do this first, l is deallocated */
!
! /* Indexes are dropped during heap drop */
if (temp_rel->relkind != RELKIND_INDEX)
- {
- char relname[NAMEDATALEN];
-
- /* safe from deallocation */
- strcpy(relname, temp_rel->user_relname);
heap_drop_with_catalog(relname, allowSystemTableMods);
! }
!
! l = next;
}
- temp_rels = NIL;
CommitTransactionCommand();
}
--- 91,112 ----
void
remove_all_temp_relations(void)
{
AbortOutOfAnyTransaction();
StartTransactionCommand();
! while (temp_rels != NIL)
{
! char relname[NAMEDATALEN];
! TempTable *temp_rel = (TempTable *) lfirst(temp_rels);
! /* safe from deallocation */
! strcpy(relname, temp_rel->user_relname);
if (temp_rel->relkind != RELKIND_INDEX)
heap_drop_with_catalog(relname, allowSystemTableMods);
! else
! index_drop(temp_rel->relid);
! CommandCounterIncrement();
}
CommitTransactionCommand();
}
***************
*** 129,135 ****
* we don't have the relname for indexes, so we just pass the oid
*/
void
! remove_temp_relation(Oid relid)
{
MemoryContext oldcxt;
List *l,
--- 116,122 ----
* we don't have the relname for indexes, so we just pass the oid
*/
void
! remove_temp_rel_by_relid(Oid relid)
{
MemoryContext oldcxt;
List *l,
***************
*** 179,185 ****
* We just have to delete the map entry.
*/
void
! invalidate_temp_relations(void)
{
MemoryContext oldcxt;
List *l,
--- 166,172 ----
* We just have to delete the map entry.
*/
void
! remove_temp_rel_in_myxid(void)
{
MemoryContext oldcxt;
List *l,
Index: src/include/utils/temprel.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/utils/temprel.h,v
retrieving revision 1.10
diff -c -r1.10 temprel.h
*** src/include/utils/temprel.h 2000/06/20 06:41:11 1.10
--- src/include/utils/temprel.h 2000/10/11 21:23:14
***************
*** 18,29 ****
extern void create_temp_relation(const char *relname,
HeapTuple
pg_class_tuple);
! extern void remove_temp_relation(Oid relid);
extern bool rename_temp_relation(const char *oldname,
const char *newname);
extern void remove_all_temp_relations(void);
! extern void invalidate_temp_relations(void);
extern char *get_temp_rel_by_username(const char *user_relname);
extern char *get_temp_rel_by_physicalname(const char *relname);
--- 18,29 ----
extern void create_temp_relation(const char *relname,
HeapTuple
pg_class_tuple);
! extern void remove_temp_rel_by_relid(Oid relid);
extern bool rename_temp_relation(const char *oldname,
const char *newname);
extern void remove_all_temp_relations(void);
! extern void remove_temp_rel_in_myxid(void);
extern char *get_temp_rel_by_username(const char *user_relname);
extern char *get_temp_rel_by_physicalname(const char *relname);
