From c74eb6add12ab76c07e2e18c35d52e2709364ff4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 22 Dec 2000 23:12:07 +0000 Subject: [PATCH] Small cleanup of temp-table handling. Disallow creation of a non-temp table that inherits from a temp table. Make sure the right things happen if one creates a temp table, creates another temp that inherits from it, then renames the first one. (Previously, system would end up trying to delete the temp tables in the wrong order.) --- src/backend/catalog/heap.c | 6 +++--- src/backend/catalog/index.c | 4 ++-- src/backend/commands/command.c | 5 ++--- src/backend/commands/creatinh.c | 13 +++++++++---- src/backend/commands/vacuum.c | 4 ++-- src/backend/utils/cache/relcache.c | 2 +- src/backend/utils/cache/temprel.c | 18 +++++++++++++----- src/include/utils/temprel.h | 2 ++ 8 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 840112df00..23cd9ac109 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -801,7 +801,7 @@ heap_create_with_catalog(char *relname, /* temp tables can mask non-temp tables */ if ((!istemp && RelnameFindRelid(relname)) || - (istemp && get_temp_rel_by_username(relname) != NULL)) + (istemp && is_temp_rel_name(relname))) elog(ERROR, "Relation '%s' already exists", relname); if (istemp) @@ -813,7 +813,7 @@ heap_create_with_catalog(char *relname, } /* ---------------- - * get_temp_rel_by_username() couldn't check the simultaneous + * RelnameFindRelid couldn't detect simultaneous * creation. Uniqueness will be really checked by unique * indexes of system tables but we couldn't check it here. * We have to postpone creating the disk file for this @@ -1404,7 +1404,7 @@ heap_drop_with_catalog(const char *relname, Relation rel; Oid rid; bool has_toasttable; - bool istemp = (get_temp_rel_by_username(relname) != NULL); + bool istemp = is_temp_rel_name(relname); int i; /* ---------------- diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index e1478e5230..bab33d44f1 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -145,7 +145,7 @@ GetHeapRelationOid(char *heapRelationName, char *indexRelationName, bool istemp) indoid = RelnameFindRelid(indexRelationName); if ((!istemp && OidIsValid(indoid)) || - (istemp && get_temp_rel_by_username(indexRelationName) != NULL)) + (istemp && is_temp_rel_name(indexRelationName))) elog(ERROR, "Cannot create index: '%s' already exists", indexRelationName); @@ -885,7 +885,7 @@ index_create(char *heapRelationName, TupleDesc indexTupDesc; Oid heapoid; Oid indexoid; - bool istemp = (get_temp_rel_by_username(heapRelationName) != NULL); + bool istemp = is_temp_rel_name(heapRelationName); char *temp_relname = NULL; SetReindexProcessing(false); diff --git a/src/backend/commands/command.c b/src/backend/commands/command.c index 7ad51f27db..958f20399d 100644 --- a/src/backend/commands/command.c +++ b/src/backend/commands/command.c @@ -1237,10 +1237,9 @@ AlterTableAddConstraint(char *relationName, int i; bool found = false; - if (get_temp_rel_by_username(fkconstraint->pktable_name)!=NULL && - get_temp_rel_by_username(relationName)==NULL) { + if (is_temp_rel_name(fkconstraint->pktable_name) && + !is_temp_rel_name(relationName)) elog(ERROR, "ALTER TABLE / ADD CONSTRAINT: Unable to reference temporary table from permanent table constraint."); - } /* * Grab an exclusive lock on the pk table, so that someone diff --git a/src/backend/commands/creatinh.c b/src/backend/commands/creatinh.c index 2fdcd28464..104e791934 100644 --- a/src/backend/commands/creatinh.c +++ b/src/backend/commands/creatinh.c @@ -24,8 +24,9 @@ #include "catalog/pg_type.h" #include "commands/creatinh.h" #include "miscadmin.h" -#include "utils/syscache.h" #include "optimizer/clauses.h" +#include "utils/syscache.h" +#include "utils/temprel.h" /* ---------------- * local stuff @@ -34,7 +35,7 @@ static int checkAttrExists(const char *attributeName, const char *attributeType, List *schema); -static List *MergeAttributes(List *schema, List *supers, +static List *MergeAttributes(List *schema, List *supers, bool istemp, List **supOids, List **supconstr); static void StoreCatalogInheritance(Oid relationId, List *supers); static void setRelhassubclassInRelation(Oid relationId, bool relhassubclass); @@ -71,7 +72,7 @@ DefineRelation(CreateStmt *stmt, char relkind) * including inherited attributes. * ---------------- */ - schema = MergeAttributes(schema, stmt->inhRelnames, + schema = MergeAttributes(schema, stmt->inhRelnames, stmt->istemp, &inheritOids, &old_constraints); numberOfAttributes = length(schema); @@ -283,6 +284,7 @@ change_varattnos_of_a_node(Node *node, const AttrNumber *newattno) * 'schema' is the column/attribute definition for the table. (It's a list * of ColumnDef's.) It is destructively changed. * 'supers' is a list of names (as Value objects) of parent relations. + * 'istemp' is TRUE if we are creating a temp relation. * * Output arguments: * 'supOids' receives an integer list of the OIDs of the parent relations. @@ -311,7 +313,7 @@ change_varattnos_of_a_node(Node *node, const AttrNumber *newattno) * stud_emp {7:percent} */ static List * -MergeAttributes(List *schema, List *supers, +MergeAttributes(List *schema, List *supers, bool istemp, List **supOids, List **supconstr) { List *entry; @@ -378,6 +380,9 @@ MergeAttributes(List *schema, List *supers, if (relation->rd_rel->relkind != RELKIND_RELATION) elog(ERROR, "CREATE TABLE: inherited relation \"%s\" is not a table", name); + /* Permanent rels cannot inherit from temporary ones */ + if (!istemp && is_temp_rel_name(name)) + elog(ERROR, "CREATE TABLE: cannot inherit from temp relation \"%s\"", name); parentOids = lappendi(parentOids, relation->rd_id); setRelhassubclassInRelation(relation->rd_id, true); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 0915d44512..ffed0d3b96 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -274,8 +274,8 @@ getrels(NameData *VacRelP) char *nontemp_relname; /* We must re-map temp table names bjm 2000-04-06 */ - if ((nontemp_relname = - get_temp_rel_by_username(NameStr(*VacRelP))) == NULL) + nontemp_relname = get_temp_rel_by_username(NameStr(*VacRelP)); + if (nontemp_relname == NULL) nontemp_relname = NameStr(*VacRelP); ScanKeyEntryInitialize(&key, 0x0, Anum_pg_class_relname, diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 9157050d19..e795768ed1 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1526,7 +1526,7 @@ RelationNameGetRelation(const char *relationName) * ---------------- */ temprelname = get_temp_rel_by_username(relationName); - if (temprelname) + if (temprelname != NULL) relationName = temprelname; /* ---------------- diff --git a/src/backend/utils/cache/temprel.c b/src/backend/utils/cache/temprel.c index cbdecfd639..c5c7ef680d 100644 --- a/src/backend/utils/cache/temprel.c +++ b/src/backend/utils/cache/temprel.c @@ -152,13 +152,19 @@ rename_temp_relation(const char *oldname, continue; /* ignore non-matching entries */ /* We are renaming a temp table --- is it OK to do so? */ - if (get_temp_rel_by_username(newname) != NULL) + if (is_temp_rel_name(newname)) elog(ERROR, "Cannot rename temp table \"%s\": temp table \"%s\" already exists", oldname, newname); /* * Create a new mapping entry and mark the old one deleted in this * xact. One of these entries will be deleted at xact end. + * + * NOTE: the new mapping entry is inserted into the list just after + * the old one. We could alternatively insert it before the old one, + * but that'd take more code. It does need to be in one spot or the + * other, to ensure that deletion of temp rels happens in the right + * order during remove_all_temp_relations(). */ oldcxt = MemoryContextSwitchTo(CacheMemoryContext); @@ -168,7 +174,7 @@ rename_temp_relation(const char *oldname, StrNCpy(NameStr(new_temp_rel->user_relname), newname, NAMEDATALEN); new_temp_rel->created_in_cur_xact = true; - temp_rels = lcons(new_temp_rel, temp_rels); + lnext(l) = lcons(new_temp_rel, lnext(l)); temp_rel->deleted_in_cur_xact = true; @@ -178,7 +184,7 @@ rename_temp_relation(const char *oldname, } /* Old name does not match any temp table name, what about new? */ - if (get_temp_rel_by_username(newname) != NULL) + if (is_temp_rel_name(newname)) elog(ERROR, "Cannot rename \"%s\" to \"%s\": a temp table by that name already exists", oldname, newname); @@ -205,7 +211,8 @@ remove_all_temp_relations(void) * Scan the list and delete all entries not already deleted. * We need not worry about list entries getting deleted from under us, * because remove_temp_rel_by_relid() doesn't remove entries, only - * mark them dead. + * mark them dead. Note that entries will be deleted in reverse order + * of creation --- that's critical for cases involving inheritance. */ foreach(l, temp_rels) { @@ -286,7 +293,8 @@ AtEOXact_temp_relations(bool isCommit) /* * Map user name to physical name --- returns NULL if no entry. * - * This is the normal way to test whether a name is a temp table name. + * This also supports testing whether a name is a temp table name; + * see is_temp_rel_name() macro. */ char * get_temp_rel_by_username(const char *user_relname) diff --git a/src/include/utils/temprel.h b/src/include/utils/temprel.h index 9981e2ea9f..617eb64a04 100644 --- a/src/include/utils/temprel.h +++ b/src/include/utils/temprel.h @@ -28,4 +28,6 @@ extern void AtEOXact_temp_relations(bool isCommit); extern char *get_temp_rel_by_username(const char *user_relname); extern char *get_temp_rel_by_physicalname(const char *relname); +#define is_temp_rel_name(relname) (get_temp_rel_by_username(relname) != NULL) + #endif /* TEMPREL_H */ -- 2.39.5