From bdca82f44d0e0168dece56cbd53b54ba142f328f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 22 Feb 2011 19:23:23 -0500 Subject: [PATCH] Add a relkind field to RangeTblEntry to avoid some syscache lookups. The recent additions for FDW support required checking foreign-table-ness in several places in the parse/plan chain. While it's not clear whether that would really result in a noticeable slowdown, it seems best to avoid any performance risk by keeping a copy of the relation's relkind in RangeTblEntry. That might have some other uses later, anyway. Per discussion. --- src/backend/catalog/dependency.c | 1 + src/backend/commands/copy.c | 1 + src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/nodeFuncs.c | 2 - src/backend/nodes/outfuncs.c | 2 +- src/backend/nodes/print.c | 8 +--- src/backend/nodes/readfuncs.c | 2 +- src/backend/optimizer/path/allpaths.c | 67 ++++++++++++++------------- src/backend/optimizer/plan/planner.c | 3 +- src/backend/parser/analyze.c | 23 +++------ src/backend/parser/parse_relation.c | 2 + src/backend/parser/parse_target.c | 2 - src/backend/rewrite/rewriteHandler.c | 9 +++- src/backend/utils/adt/ri_triggers.c | 2 + src/backend/utils/adt/ruleutils.c | 7 ++- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_class.h | 17 +++---- src/include/nodes/parsenodes.h | 7 ++- 19 files changed, 85 insertions(+), 74 deletions(-) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 1679776f01..bce0e07683 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1284,6 +1284,7 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender, rte.type = T_RangeTblEntry; rte.rtekind = RTE_RELATION; rte.relid = relId; + rte.relkind = RELKIND_RELATION; /* no need for exactness here */ context.rtables = list_make1(list_make1(&rte)); diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 44f568f396..3c504e96be 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -763,6 +763,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString) rte = makeNode(RangeTblEntry); rte->rtekind = RTE_RELATION; rte->relid = RelationGetRelid(rel); + rte->relkind = rel->rd_rel->relkind; rte->requiredPerms = required_access; tupDesc = RelationGetDescr(rel); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 1ba746bb4d..04763d44eb 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1927,6 +1927,7 @@ _copyRangeTblEntry(RangeTblEntry *from) COPY_SCALAR_FIELD(rtekind); COPY_SCALAR_FIELD(relid); + COPY_SCALAR_FIELD(relkind); COPY_NODE_FIELD(subquery); COPY_SCALAR_FIELD(jointype); COPY_NODE_FIELD(joinaliasvars); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index dd332f19f0..c896f49ff6 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2286,6 +2286,7 @@ _equalRangeTblEntry(RangeTblEntry *a, RangeTblEntry *b) { COMPARE_SCALAR_FIELD(rtekind); COMPARE_SCALAR_FIELD(relid); + COMPARE_SCALAR_FIELD(relkind); COMPARE_NODE_FIELD(subquery); COMPARE_SCALAR_FIELD(jointype); COMPARE_NODE_FIELD(joinaliasvars); diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 8a23047d38..d4b9242917 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -1671,7 +1671,6 @@ range_table_walker(List *rtable, switch (rte->rtekind) { case RTE_RELATION: - case RTE_SPECIAL: case RTE_CTE: /* nothing to do */ break; @@ -2374,7 +2373,6 @@ range_table_mutator(List *rtable, switch (rte->rtekind) { case RTE_RELATION: - case RTE_SPECIAL: case RTE_CTE: /* we don't bother to copy eref, aliases, etc; OK? */ break; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 10f630e27f..706b2425cf 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2275,8 +2275,8 @@ _outRangeTblEntry(StringInfo str, RangeTblEntry *node) switch (node->rtekind) { case RTE_RELATION: - case RTE_SPECIAL: WRITE_OID_FIELD(relid); + WRITE_CHAR_FIELD(relkind); break; case RTE_SUBQUERY: WRITE_NODE_FIELD(subquery); diff --git a/src/backend/nodes/print.c b/src/backend/nodes/print.c index 14487f25a1..cd119dbabb 100644 --- a/src/backend/nodes/print.c +++ b/src/backend/nodes/print.c @@ -265,8 +265,8 @@ print_rt(List *rtable) switch (rte->rtekind) { case RTE_RELATION: - printf("%d\t%s\t%u", - i, rte->eref->aliasname, rte->relid); + printf("%d\t%s\t%u\t%c", + i, rte->eref->aliasname, rte->relid, rte->relkind); break; case RTE_SUBQUERY: printf("%d\t%s\t[subquery]", @@ -276,10 +276,6 @@ print_rt(List *rtable) printf("%d\t%s\t[join]", i, rte->eref->aliasname); break; - case RTE_SPECIAL: - printf("%d\t%s\t[special]", - i, rte->eref->aliasname); - break; case RTE_FUNCTION: printf("%d\t%s\t[rangefunction]", i, rte->eref->aliasname); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index b007caeee3..c76884e991 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1171,8 +1171,8 @@ _readRangeTblEntry(void) switch (local_node->rtekind) { case RTE_RELATION: - case RTE_SPECIAL: READ_OID_FIELD(relid); + READ_CHAR_FIELD(relkind); break; case RTE_SUBQUERY: READ_NODE_FIELD(subquery); diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index c835a954ed..dc2a23bb27 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -176,41 +176,44 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, /* It's an "append relation", process accordingly */ set_append_rel_pathlist(root, rel, rti, rte); } - else if (rel->rtekind == RTE_SUBQUERY) - { - /* Subquery --- generate a separate plan for it */ - set_subquery_pathlist(root, rel, rti, rte); - } - else if (rel->rtekind == RTE_FUNCTION) - { - /* RangeFunction --- generate a suitable path for it */ - set_function_pathlist(root, rel, rte); - } - else if (rel->rtekind == RTE_VALUES) - { - /* Values list --- generate a suitable path for it */ - set_values_pathlist(root, rel, rte); - } - else if (rel->rtekind == RTE_CTE) - { - /* CTE reference --- generate a suitable path for it */ - if (rte->self_reference) - set_worktable_pathlist(root, rel, rte); - else - set_cte_pathlist(root, rel, rte); - } else { - Assert(rel->rtekind == RTE_RELATION); - if (get_rel_relkind(rte->relid) == RELKIND_FOREIGN_TABLE) + switch (rel->rtekind) { - /* Foreign table */ - set_foreign_pathlist(root, rel, rte); - } - else - { - /* Plain relation */ - set_plain_rel_pathlist(root, rel, rte); + case RTE_RELATION: + if (rte->relkind == RELKIND_FOREIGN_TABLE) + { + /* Foreign table */ + set_foreign_pathlist(root, rel, rte); + } + else + { + /* Plain relation */ + set_plain_rel_pathlist(root, rel, rte); + } + break; + case RTE_SUBQUERY: + /* Subquery --- generate a separate plan for it */ + set_subquery_pathlist(root, rel, rti, rte); + break; + case RTE_FUNCTION: + /* RangeFunction --- generate a suitable path for it */ + set_function_pathlist(root, rel, rte); + break; + case RTE_VALUES: + /* Values list --- generate a suitable path for it */ + set_values_pathlist(root, rel, rte); + break; + case RTE_CTE: + /* CTE reference --- generate a suitable path for it */ + if (rte->self_reference) + set_worktable_pathlist(root, rel, rte); + else + set_cte_pathlist(root, rel, rte); + break; + default: + elog(ERROR, "unexpected rtekind: %d", (int) rel->rtekind); + break; } } diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index b73b872b31..ee09673051 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1915,7 +1915,7 @@ preprocess_rowmarks(PlannerInfo *root) newrc->rowmarkId = ++(root->glob->lastRowMarkId); /* real tables support REFERENCE, anything else needs COPY */ if (rte->rtekind == RTE_RELATION && - get_rel_relkind(rte->relid) != RELKIND_FOREIGN_TABLE) + rte->relkind != RELKIND_FOREIGN_TABLE) newrc->markType = ROW_MARK_REFERENCE; else newrc->markType = ROW_MARK_COPY; @@ -3078,6 +3078,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid) rte = makeNode(RangeTblEntry); rte->rtekind = RTE_RELATION; rte->relid = tableOid; + rte->relkind = RELKIND_RELATION; rte->inh = false; rte->inFromCl = true; query->rtable = list_make1(rte); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index c7a7a23076..7f28d9df4f 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -40,7 +40,6 @@ #include "parser/parse_target.h" #include "parser/parsetree.h" #include "rewrite/rewriteManip.h" -#include "utils/lsyscache.h" #include "utils/rel.h" @@ -2178,13 +2177,11 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, { case RTE_RELATION: /* ignore foreign tables */ - if (get_rel_relkind(rte->relid) != RELKIND_FOREIGN_TABLE) - { - applyLockingClause(qry, i, - lc->forUpdate, lc->noWait, - pushedDown); - rte->requiredPerms |= ACL_SELECT_FOR_UPDATE; - } + if (rte->relkind == RELKIND_FOREIGN_TABLE) + break; + applyLockingClause(qry, i, + lc->forUpdate, lc->noWait, pushedDown); + rte->requiredPerms |= ACL_SELECT_FOR_UPDATE; break; case RTE_SUBQUERY: applyLockingClause(qry, i, @@ -2231,11 +2228,11 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, switch (rte->rtekind) { case RTE_RELATION: - if (get_rel_relkind(rte->relid) == RELKIND_FOREIGN_TABLE) + if (rte->relkind == RELKIND_FOREIGN_TABLE) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("SELECT FOR UPDATE/SHARE cannot be used with foreign table \"%s\"", - get_rel_name(rte->relid)), + rte->eref->aliasname), parser_errposition(pstate, thisrel->location))); applyLockingClause(qry, i, lc->forUpdate, lc->noWait, @@ -2256,12 +2253,6 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, errmsg("SELECT FOR UPDATE/SHARE cannot be applied to a join"), parser_errposition(pstate, thisrel->location))); break; - case RTE_SPECIAL: - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("SELECT FOR UPDATE/SHARE cannot be applied to NEW or OLD"), - parser_errposition(pstate, thisrel->location))); - break; case RTE_FUNCTION: ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 497c726f31..033ed411fd 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -894,6 +894,7 @@ addRangeTableEntry(ParseState *pstate, lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock; rel = parserOpenTable(pstate, relation, lockmode); rte->relid = RelationGetRelid(rel); + rte->relkind = rel->rd_rel->relkind; /* * Build the list of effective column names using user-supplied aliases @@ -956,6 +957,7 @@ addRangeTableEntryForRelation(ParseState *pstate, rte->rtekind = RTE_RELATION; rte->alias = alias; rte->relid = RelationGetRelid(rel); + rte->relkind = rel->rd_rel->relkind; /* * Build the list of effective column names using user-supplied aliases diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index a0761da875..e9ace37e2d 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -305,7 +305,6 @@ markTargetListOrigin(ParseState *pstate, TargetEntry *tle, markTargetListOrigin(pstate, tle, aliasvar, netlevelsup); } break; - case RTE_SPECIAL: case RTE_FUNCTION: case RTE_VALUES: /* not a simple relation, leave it unmarked */ @@ -1357,7 +1356,6 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup) switch (rte->rtekind) { case RTE_RELATION: - case RTE_SPECIAL: case RTE_VALUES: /* diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 3a50642fce..c0d25b15c6 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -144,6 +144,13 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown) lockmode = AccessShareLock; rel = heap_open(rte->relid, lockmode); + + /* + * While we have the relation open, update the RTE's relkind, + * just in case it changed since this rule was made. + */ + rte->relkind = rel->rd_rel->relkind; + heap_close(rel, NoLock); break; @@ -1393,7 +1400,7 @@ markQueryForLocking(Query *qry, Node *jtnode, if (rte->rtekind == RTE_RELATION) { /* ignore foreign tables */ - if (get_rel_relkind(rte->relid) != RELKIND_FOREIGN_TABLE) + if (rte->relkind != RELKIND_FOREIGN_TABLE) { applyLockingClause(qry, rti, forUpdate, noWait, pushedDown); rte->requiredPerms |= ACL_SELECT_FOR_UPDATE; diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 5ef1563d1c..591d2eb16b 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2651,11 +2651,13 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) pkrte = makeNode(RangeTblEntry); pkrte->rtekind = RTE_RELATION; pkrte->relid = RelationGetRelid(pk_rel); + pkrte->relkind = pk_rel->rd_rel->relkind; pkrte->requiredPerms = ACL_SELECT; fkrte = makeNode(RangeTblEntry); fkrte->rtekind = RTE_RELATION; fkrte->relid = RelationGetRelid(fk_rel); + fkrte->relkind = fk_rel->rd_rel->relkind; fkrte->requiredPerms = ACL_SELECT; for (i = 0; i < riinfo.nkeys; i++) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index cd64235438..d9b359465a 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -628,6 +628,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) if (!isnull) { Node *qual; + char relkind; deparse_context context; deparse_namespace dpns; RangeTblEntry *oldrte; @@ -637,10 +638,13 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) qual = stringToNode(TextDatumGetCString(value)); + relkind = get_rel_relkind(trigrec->tgrelid); + /* Build minimal OLD and NEW RTEs for the rel */ oldrte = makeNode(RangeTblEntry); oldrte->rtekind = RTE_RELATION; oldrte->relid = trigrec->tgrelid; + oldrte->relkind = relkind; oldrte->eref = makeAlias("old", NIL); oldrte->inh = false; oldrte->inFromCl = true; @@ -648,6 +652,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) newrte = makeNode(RangeTblEntry); newrte->rtekind = RTE_RELATION; newrte->relid = trigrec->tgrelid; + newrte->relkind = relkind; newrte->eref = makeAlias("new", NIL); newrte->inh = false; newrte->inFromCl = true; @@ -2125,6 +2130,7 @@ deparse_context_for(const char *aliasname, Oid relid) rte = makeNode(RangeTblEntry); rte->rtekind = RTE_RELATION; rte->relid = relid; + rte->relkind = RELKIND_RELATION; /* no need for exactness here */ rte->eref = makeAlias(aliasname, NIL); rte->inh = false; rte->inFromCl = true; @@ -4004,7 +4010,6 @@ get_name_for_var_field(Var *var, int fieldno, switch (rte->rtekind) { case RTE_RELATION: - case RTE_SPECIAL: case RTE_VALUES: /* diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 70106414cf..0a0544bc38 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201102221 +#define CATALOG_VERSION_NO 201102222 #endif diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h index 42bc8635a2..e103530a3f 100644 --- a/src/include/catalog/pg_class.h +++ b/src/include/catalog/pg_class.h @@ -49,7 +49,7 @@ CATALOG(pg_class,1259) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83) BKI_SCHEMA_MACRO Oid reltoastidxid; /* if toast table, OID of chunk_id index */ bool relhasindex; /* T if has (or has had) any indexes */ bool relisshared; /* T if shared across databases */ - char relpersistence; /* see RELPERSISTENCE_xxx constants */ + char relpersistence; /* see RELPERSISTENCE_xxx constants below */ char relkind; /* see RELKIND_xxx constants below */ int2 relnatts; /* number of user attributes */ @@ -139,17 +139,18 @@ DESCR(""); DATA(insert OID = 1259 ( pg_class PGNSP 83 0 PGUID 0 0 0 0 0 0 0 f f p r 26 0 t f f f f 3 _null_ _null_ )); DESCR(""); + +#define RELKIND_RELATION 'r' /* ordinary table */ #define RELKIND_INDEX 'i' /* secondary index */ -#define RELKIND_RELATION 'r' /* ordinary cataloged heap */ -#define RELKIND_SEQUENCE 'S' /* SEQUENCE relation */ -#define RELKIND_UNCATALOGED 'u' /* temporary heap */ -#define RELKIND_TOASTVALUE 't' /* moved off huge values */ +#define RELKIND_SEQUENCE 'S' /* sequence object */ +#define RELKIND_TOASTVALUE 't' /* for out-of-line values */ #define RELKIND_VIEW 'v' /* view */ #define RELKIND_COMPOSITE_TYPE 'c' /* composite type */ #define RELKIND_FOREIGN_TABLE 'f' /* foreign table */ +#define RELKIND_UNCATALOGED 'u' /* not yet cataloged */ -#define RELPERSISTENCE_PERMANENT 'p' -#define RELPERSISTENCE_UNLOGGED 'u' -#define RELPERSISTENCE_TEMP 't' +#define RELPERSISTENCE_PERMANENT 'p' /* regular table */ +#define RELPERSISTENCE_UNLOGGED 'u' /* unlogged permanent table */ +#define RELPERSISTENCE_TEMP 't' /* temporary table */ #endif /* PG_CLASS_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 63a61e3da2..536c03245e 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -597,6 +597,9 @@ typedef struct XmlSerialize * like outer joins and join-output-column aliasing.) Other special * RTE types also exist, as indicated by RTEKind. * + * Note that we consider RTE_RELATION to cover anything that has a pg_class + * entry. relkind distinguishes the sub-cases. + * * alias is an Alias node representing the AS alias-clause attached to the * FROM expression, or NULL if no clause. * @@ -643,7 +646,7 @@ typedef struct XmlSerialize * indicates no permissions checking). If checkAsUser is not zero, * then do the permissions checks using the access rights of that user, * not the current effective user ID. (This allows rules to act as - * setuid gateways.) + * setuid gateways.) Permissions checks only apply to RELATION RTEs. * * For SELECT/INSERT/UPDATE permissions, if the user doesn't have * table-wide permissions then it is sufficient to have the permissions @@ -660,7 +663,6 @@ typedef enum RTEKind RTE_RELATION, /* ordinary relation reference */ RTE_SUBQUERY, /* subquery in FROM */ RTE_JOIN, /* join */ - RTE_SPECIAL, /* special rule relation (NEW or OLD) */ RTE_FUNCTION, /* function in FROM */ RTE_VALUES, /* VALUES (), (), ... */ RTE_CTE /* common table expr (WITH list element) */ @@ -682,6 +684,7 @@ typedef struct RangeTblEntry * Fields valid for a plain relation RTE (else zero): */ Oid relid; /* OID of the relation */ + char relkind; /* relation kind (see pg_class.relkind) */ /* * Fields valid for a subquery RTE (else NULL):