On 23.02.24 16:19, Tom Lane wrote:
Dean Rasheed <dean.a.rash...@gmail.com> writes:
On Fri, 23 Feb 2024 at 14:35, Peter Eisentraut <pe...@eisentraut.org> wrote:
Various code comments say that the RangeTblEntry field inh may only be
set for entries of kind RTE_RELATION.
Yes, it's explained a bit more clearly/accurately in expand_inherited_rtentry():
* "inh" is only allowed in two cases: RELATION and SUBQUERY RTEs.
Yes. The latter has been accurate for a very long time, so I'm
surprised that there are any places that think otherwise. We need
to fix them --- where did you see this exactly?
In nodes/parsenodes.h, it says both
This *must* be false for RTEs other than RTE_RELATION entries.
and also puts it under
Fields valid in all RTEs:
which are both wrong on opposite ends of the spectrum.
I think it would make more sense to group inh under "Fields valid for a
plain relation RTE" and then explain the exception for subqueries, like
it is done for several other fields.
See attached patch for a proposal. (I also shuffled a few fields around
to make the order a bit more logical.)From 631fa6a211f7d2022e78e5683ed234a00ae144bd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 29 Feb 2024 13:06:48 +0100
Subject: [PATCH] Fix description and grouping of RangeTblEntry.inh
---
src/backend/nodes/outfuncs.c | 5 +++--
src/backend/nodes/queryjumblefuncs.c | 2 +-
src/backend/nodes/readfuncs.c | 5 +++--
src/backend/parser/parse_relation.c | 11 ++---------
src/include/nodes/parsenodes.h | 15 +++++++++------
5 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 2c30bba212..1c66be3af8 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -505,8 +505,9 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
WRITE_OID_FIELD(relid);
WRITE_CHAR_FIELD(relkind);
WRITE_INT_FIELD(rellockmode);
- WRITE_NODE_FIELD(tablesample);
WRITE_UINT_FIELD(perminfoindex);
+ WRITE_BOOL_FIELD(inh);
+ WRITE_NODE_FIELD(tablesample);
break;
case RTE_SUBQUERY:
WRITE_NODE_FIELD(subquery);
@@ -516,6 +517,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
WRITE_CHAR_FIELD(relkind);
WRITE_INT_FIELD(rellockmode);
WRITE_UINT_FIELD(perminfoindex);
+ WRITE_BOOL_FIELD(inh);
break;
case RTE_JOIN:
WRITE_ENUM_FIELD(jointype, JoinType);
@@ -564,7 +566,6 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
}
WRITE_BOOL_FIELD(lateral);
- WRITE_BOOL_FIELD(inh);
WRITE_BOOL_FIELD(inFromCl);
WRITE_NODE_FIELD(securityQuals);
}
diff --git a/src/backend/nodes/queryjumblefuncs.c
b/src/backend/nodes/queryjumblefuncs.c
index 82f725baaa..1900b8bca4 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -364,8 +364,8 @@ _jumbleRangeTblEntry(JumbleState *jstate, Node *node)
{
case RTE_RELATION:
JUMBLE_FIELD(relid);
- JUMBLE_NODE(tablesample);
JUMBLE_FIELD(inh);
+ JUMBLE_NODE(tablesample);
break;
case RTE_SUBQUERY:
JUMBLE_NODE(subquery);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index b1e2f2b440..b80441e0c2 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -359,8 +359,9 @@ _readRangeTblEntry(void)
READ_OID_FIELD(relid);
READ_CHAR_FIELD(relkind);
READ_INT_FIELD(rellockmode);
- READ_NODE_FIELD(tablesample);
READ_UINT_FIELD(perminfoindex);
+ READ_BOOL_FIELD(inh);
+ READ_NODE_FIELD(tablesample);
break;
case RTE_SUBQUERY:
READ_NODE_FIELD(subquery);
@@ -370,6 +371,7 @@ _readRangeTblEntry(void)
READ_CHAR_FIELD(relkind);
READ_INT_FIELD(rellockmode);
READ_UINT_FIELD(perminfoindex);
+ READ_BOOL_FIELD(inh);
break;
case RTE_JOIN:
READ_ENUM_FIELD(jointype, JoinType);
@@ -428,7 +430,6 @@ _readRangeTblEntry(void)
}
READ_BOOL_FIELD(lateral);
- READ_BOOL_FIELD(inh);
READ_BOOL_FIELD(inFromCl);
READ_NODE_FIELD(securityQuals);
diff --git a/src/backend/parser/parse_relation.c
b/src/backend/parser/parse_relation.c
index 34a0ec5901..9a62c822e1 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1502,6 +1502,7 @@ addRangeTableEntry(ParseState *pstate,
rte->relid = RelationGetRelid(rel);
rte->relkind = rel->rd_rel->relkind;
rte->rellockmode = lockmode;
+ rte->inh = inh;
/*
* Build the list of effective column names using user-supplied aliases
@@ -1517,7 +1518,6 @@ addRangeTableEntry(ParseState *pstate,
* which is the right thing for all except target tables.
*/
rte->lateral = false;
- rte->inh = inh;
rte->inFromCl = inFromCl;
perminfo = addRTEPermissionInfo(&pstate->p_rteperminfos, rte);
@@ -1587,6 +1587,7 @@ addRangeTableEntryForRelation(ParseState *pstate,
rte->relid = RelationGetRelid(rel);
rte->relkind = rel->rd_rel->relkind;
rte->rellockmode = lockmode;
+ rte->inh = inh;
/*
* Build the list of effective column names using user-supplied aliases
@@ -1602,7 +1603,6 @@ addRangeTableEntryForRelation(ParseState *pstate,
* which is the right thing for all except target tables.
*/
rte->lateral = false;
- rte->inh = inh;
rte->inFromCl = inFromCl;
perminfo = addRTEPermissionInfo(&pstate->p_rteperminfos, rte);
@@ -1700,7 +1700,6 @@ addRangeTableEntryForSubquery(ParseState *pstate,
* addRTEPermissionInfo().
*/
rte->lateral = lateral;
- rte->inh = false; /* never true for subqueries */
rte->inFromCl = inFromCl;
/*
@@ -2023,7 +2022,6 @@ addRangeTableEntryForFunction(ParseState *pstate,
* ExecCheckPermissions()), so no need to perform
addRTEPermissionInfo().
*/
rte->lateral = lateral;
- rte->inh = false; /* never true for functions */
rte->inFromCl = inFromCl;
/*
@@ -2108,7 +2106,6 @@ addRangeTableEntryForTableFunc(ParseState *pstate,
* ExecCheckPermissions()), so no need to perform
addRTEPermissionInfo().
*/
rte->lateral = lateral;
- rte->inh = false; /* never true for tablefunc
RTEs */
rte->inFromCl = inFromCl;
/*
@@ -2189,7 +2186,6 @@ addRangeTableEntryForValues(ParseState *pstate,
* addRTEPermissionInfo().
*/
rte->lateral = lateral;
- rte->inh = false; /* never true for values RTEs */
rte->inFromCl = inFromCl;
/*
@@ -2280,7 +2276,6 @@ addRangeTableEntryForJoin(ParseState *pstate,
* addRTEPermissionInfo().
*/
rte->lateral = false;
- rte->inh = false; /* never true for joins */
rte->inFromCl = inFromCl;
/*
@@ -2425,7 +2420,6 @@ addRangeTableEntryForCTE(ParseState *pstate,
* addRTEPermissionInfo().
*/
rte->lateral = false;
- rte->inh = false; /* never true for subqueries */
rte->inFromCl = inFromCl;
/*
@@ -2545,7 +2539,6 @@ addRangeTableEntryForENR(ParseState *pstate,
* addRTEPermissionInfo().
*/
rte->lateral = false;
- rte->inh = false; /* never true for ENRs */
rte->inFromCl = inFromCl;
/*
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index baa6a97c7e..69dd01227a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -981,10 +981,6 @@ typedef struct PartitionCmd
* them from the joinaliasvars list, because that would affect the
attnums
* of Vars referencing the rest of the list.)
*
- * inh is true for relation references that should be expanded to include
- * inheritance children, if the rel has any. This *must* be false for
- * RTEs other than RTE_RELATION entries.
- *
* inFromCl marks those range variables that are listed in the FROM
clause.
* It's false for RTEs that are added to a query behind the scenes, such
* as the NEW and OLD variables for a rule, or the subqueries of a UNION.
@@ -1044,6 +1040,9 @@ typedef struct RangeTblEntry
* target table. We leave such RTEs with their original lockmode so as
to
* avoid getting an additional, lesser lock.
*
+ * inh is true for relation references that should be expanded to
include
+ * inheritance children, if the rel has any.
+ *
* perminfoindex is 1-based index of the RTEPermissionInfo belonging to
* this RTE in the containing struct's list of same; 0 if permissions
need
* not be checked for this RTE.
@@ -1056,6 +1055,10 @@ typedef struct RangeTblEntry
* in the query anymore, and the most expedient way to do that is to
* retain these fields from the old state of the RTE.
*
+ * As a special case, inh may also be true for RTE_SUBQUERY entries in
the
+ * planner (but not in the parser or rewriter); see
+ * expand_inherited_rtentry().
+ *
* As a special case, RTE_NAMEDTUPLESTORE can also set relid to indicate
* that the tuple format of the tuplestore is the same as the referenced
* relation. This allows plans referencing AFTER trigger transition
@@ -1064,8 +1067,9 @@ typedef struct RangeTblEntry
Oid relid; /* OID of the relation
*/
char relkind; /* relation kind (see
pg_class.relkind) */
int rellockmode; /* lock level that query
requires on the rel */
- struct TableSampleClause *tablesample; /* sampling info, or NULL */
Index perminfoindex;
+ bool inh; /* inheritance requested? */
+ struct TableSampleClause *tablesample; /* sampling info, or NULL */
/*
* Fields valid for a subquery RTE (else NULL):
@@ -1191,7 +1195,6 @@ typedef struct RangeTblEntry
Alias *alias; /* user-written alias clause,
if any */
Alias *eref; /* expanded reference names */
bool lateral; /* subquery, function, or
values is LATERAL? */
- bool inh; /* inheritance requested? */
bool inFromCl; /* present in FROM clause? */
List *securityQuals; /* security barrier quals to apply, if
any */
} RangeTblEntry;
--
2.43.2
