Обсуждение: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

Поиск
Список
Период
Сортировка

Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
I started to think a little harder about the rough ideas I sketched
yesterday in [1] about making the planner deal with outer joins in
a less ad-hoc manner.  One thing that emerged very quickly is that
I was misremembering how the parser creates join alias Vars.
Consider for example

create table t1(a int, b int);
create table t2(x int, y int);

select a, t1.a, x, t2.x from t1 left join t2 on b = y;

The Vars that the parser will produce in the SELECT's targetlist have,
respectively,

             :varno 3 
             :varattno 1 

             :varno 1 
             :varattno 1 

             :varno 3 
             :varattno 3 

             :varno 2 
             :varattno 1 

(where "3" is the rangetable index of the unnamed join relation).
So as far as the parser is concerned, a "join alias" var is just
one that you named by referencing the join output column; it's
not tracking whether the value is one that's affected by the join
semantics.

What I'd like, in order to make progress with the planner rewrite,
is that all four Vars in the tlist have varno 3, showing that
they are (potentially) semantically distinct from the Vars in
the JOIN ON clause (which'd have varnos 1 and 2 in this example).

This is a pretty small change as far as most of the system is
concerned; there should be noplace that fails to cope with a
join alias Var, since it'd have been legal to write a join
alias Var in anyplace that would change.

However, it's a bit sticky for ruleutils.c, which needs to be
able to regurgitate these Vars in their original spellings.
(This is "needs", not "wants", because there are various
conditions under which we don't have the option of spelling
it either way.  For instance, if both tables expose columns
named "z", then you must write "t1.z" or "t2.z"; the columns
won't have unique names at the join level.)

What I'd like to do about that is redefine the existing
varnoold/varoattno fields as being the "syntactic" identifier
of the Var, versus the "semantic" identifier that varno/varattno
would be, and have ruleutils.c always use varnoold/varoattno
when trying to print a Var.

I think that this approach would greatly clarify what those fields
mean and how they should be manipulated --- for example, it makes
it clear that _equalVar() should ignore varnoold/varoattno, since
Vars with the same semantic meaning should be considered equal
even if they were spelled differently.

While at it, I'd be inclined to rename those fields, since the
existing names aren't even consistently spelled, much less meaningful.
Perhaps "varsno/varsattno" or "varnosyn/varattnosyn".

Thoughts?

            regards, tom lane

[1] https://www.postgresql.org/message-id/7771.1576452845%40sss.pgh.pa.us



Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Robert Haas
Дата:
On Mon, Dec 16, 2019 at 12:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> What I'd like, in order to make progress with the planner rewrite,
> is that all four Vars in the tlist have varno 3, showing that
> they are (potentially) semantically distinct from the Vars in
> the JOIN ON clause (which'd have varnos 1 and 2 in this example).
>
> This is a pretty small change as far as most of the system is
> concerned; there should be noplace that fails to cope with a
> join alias Var, since it'd have been legal to write a join
> alias Var in anyplace that would change.

I don't have an opinion about the merits of this change, but I'm
curious how this manages to work. It seems like there would be a fair
number of places that needed to map the join alias var back to some
baserel that can supply it. And it seems like there could be multiple
levels of join alias vars as well, since you could have joins nested
inside of other joins, possibly with subqueries involved.

At some point I had the idea that it might make sense to have
equivalence classes that had both a list of full members (which are
exactly equivalent) and nullable members (which are either equivalent
or null). I'm not sure whether that idea is of any practical use,
though. It does seems strange to me that the representation you are
proposing gets at the question only indirectly. The nullable version
of the Var has got a different varno and varattno than the
non-nullable version of the Var, but other than that there's no
connection between them. How do you go about matching those together?
I guess varnoold/varoattno can do the trick, but if that's only being
used by ruleutils.c then there must be some other mechanism.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Dec 16, 2019 at 12:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What I'd like, in order to make progress with the planner rewrite,
>> is that all four Vars in the tlist have varno 3, showing that
>> they are (potentially) semantically distinct from the Vars in
>> the JOIN ON clause (which'd have varnos 1 and 2 in this example).

> I don't have an opinion about the merits of this change, but I'm
> curious how this manages to work. It seems like there would be a fair
> number of places that needed to map the join alias var back to some
> baserel that can supply it. And it seems like there could be multiple
> levels of join alias vars as well, since you could have joins nested
> inside of other joins, possibly with subqueries involved.

Sure.  Right now, we smash join aliases down to the ultimately-referenced
base vars early in planning (see flatten_join_alias_vars).  After the
patch that I'm proposing right now, that would continue to be the case,
so there'd be little change in most of the planner from this.  However,
the later changes that I speculated about in the other thread would
involve delaying that smashing in cases where the join output value is
possibly different from the input value, so that we would have a clear
representational distinction between those things, something we lack
today.

> At some point I had the idea that it might make sense to have
> equivalence classes that had both a list of full members (which are
> exactly equivalent) and nullable members (which are either equivalent
> or null).

Yeah, this is another way that you might get at the problem, but it
seems to me it's not really addressing the fundamental squishiness.
If the "nullable members" might be null, then what semantics are
you promising exactly?  You certainly haven't got anything that
defines a sort order for them.

> I'm not sure whether that idea is of any practical use,
> though. It does seems strange to me that the representation you are
> proposing gets at the question only indirectly. The nullable version
> of the Var has got a different varno and varattno than the
> non-nullable version of the Var, but other than that there's no
> connection between them. How do you go about matching those together?

You'd have to look into the join's joinaliasvars list (or more likely,
some new planner data structure derived from that) to discover that
there's any connection.  That seems fine to me, because AFAICS
relatively few places would need to do that.  It's certainly better
than using a representation that suggests that two values are the same
when they're not.  (TBH, I've spent the last dozen years waiting for
someone to come up with an example that completely breaks equivalence
classes, if not our entire approach to outer joins.  So far we've been
able to work around every case, but we've sometimes had to give up on
optimizations that would be nice to have.)

A related example that is bugging me is that the grouping-sets patch
broke the meaning of Vars that represent post-grouping values ---
there again, the value might have gone to null as a result of grouping,
but you can't tell it apart from values that haven't.  I think this is
less critical because such Vars can't appear in FROM/WHERE so they're
of little interest to most of the planner, but we've still had to put
in kluges like 90947674f because of that.  We might be well advised
to invent some join-alias-like mechanism for those.  (I have a vague
memory now that Gierth wanted to do something like that and I
discouraged it because it was unlike the way we did outer joins ...
so he was right, but what we should have done was fix outer joins not
double down on the kluge.)

> I guess varnoold/varoattno can do the trick, but if that's only being
> used by ruleutils.c then there must be some other mechanism.

Actually, they're nothing but debug support currently --- ruleutils
doesn't use them either.  It's possible that this change would allow
ruleutils to save cycles in a lot of cases by not having to drill down
through subplans to identify the ultimate referent of upper-plan Vars.
But I haven't investigated that yet.

            regards, tom lane



Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
I wrote:
> I started to think a little harder about the rough ideas I sketched
> yesterday in [1] about making the planner deal with outer joins in
> a less ad-hoc manner.
> [1] https://www.postgresql.org/message-id/7771.1576452845%40sss.pgh.pa.us

After further study, the idea of using join alias vars to track
outer-join semantics basically doesn't work at all.  Join alias vars in
their current form (ie, references to the output columns of a JOIN RTE)
aren't suitable for the purpose of representing possibly-nulled inputs
to that same RTE.  There are two big stumbling blocks:

* In the presence of JOIN USING, we don't necessarily have a JOIN output
column that is equivalent to either input column.  The output is
definitely not equal to the nullable side of an OJ, since it won't go to
NULL; and it might not be equivalent to the non-nullable side either,
because JOIN USING might've coerced it to some common datatype.

* We also don't have any output column that could represent a whole-row
reference to either input table.  I thought about representing that with
a RowExpr of join output Vars, but that fails to preserve the existing
semantics: a whole-row reference to the nullable side goes to NULL, not
to a row of NULLs, when we're null-extending the join.

So that kind of crashed and burned.  We could maybe fake it by inventing
some new conventions about magic attnums of the join RTE that correspond
to the values we want, but that seems really messy and error-prone.

The alternatives that seem plausible at this point are

(1) Create some sort of wrapper node indicating "the contents of this
expression might be replaced by NULL".  This is basically what the
planner's PlaceHolderVars do, so maybe we'd just be talking about
introducing those at some earlier stage.

(2) Explicitly mark Vars as being nullable by some outer join.  I think
we could probably get this down to one additional integer field in
struct Var, so it wouldn't be too much of a penalty.

The wrapper approach is more general since you can wrap something
that's not necessarily a plain Var; but it's also bulkier and so
probably a bit less efficient.  I'm not sure which idea I like better.

With either approach, we could either make parse analysis inject the
nullability markings, or wait to do it in the planner.  On a purely
abstract system structural level, I like the former better: it is
exactly the province of parse analysis to decide what are the semantics
of what the user typed, and surely what a Var means is part of that.
OTOH, if we do it that way, the planner potentially has to rearrange the
markings after it does join strength reduction; so maybe it's best to
just wait till after that planning phase to address this at all.

Any thoughts about that?

Anyway, I had started to work on getting parse analysis to label
outer-join-nullable Vars properly, and soon decided that no matter how
we do it, there's not enough information available at the point where
parse analysis makes a Var.  The referenced RTE is not, in itself,
enough info, and I don't think we want to decorate RTEs with more info
that's only needed during parse analysis.  What would be saner is to add
any extra info to the ParseNamespaceItem structs.  But that requires
some refactoring to allow the ParseNamespaceItems, not just the
referenced RTEs, to be passed down through Var lookup/construction.
So attached is a patch that refactors things that way.  As proof of
concept, I added the rangetable index to ParseNamespaceItem, and used
that to get rid of the RTERangeTablePosn() searches that we formerly had
in a bunch of places.  Now, RTERangeTablePosn() isn't likely to be all
that expensive, but still this should be a little faster and cleaner.
Also, I was able to confine the fuzzy-lookup heuristic stuff to within
parse_relation.c instead of letting it bleed out to the rest of the
parser.

This seems to me to be good cleanup regardless of whether we ever
ask parse analysis to label outer-join-nullable Vars.  So, barring
objection, I'd like to push it soon.

            regards, tom lane

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 85d7a96..0656279 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1344,7 +1344,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
     int            sublist_length = -1;
     bool        lateral = false;
     RangeTblEntry *rte;
-    int            rtindex;
+    ParseNamespaceItem *nsitem;
     ListCell   *lc;
     ListCell   *lc2;
     int            i;
@@ -1516,15 +1516,15 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
                                       NULL, lateral, true);
     addRTEtoQuery(pstate, rte, true, true, true);

-    /* assume new rte is at end */
-    rtindex = list_length(pstate->p_rtable);
-    Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
+    /* grab the namespace item made by addRTEtoQuery */
+    nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace);
+    Assert(rte == nsitem->p_rte);

     /*
      * Generate a targetlist as though expanding "*"
      */
     Assert(pstate->p_next_resno == 1);
-    qry->targetList = expandRelAttrs(pstate, rte, rtindex, 0, -1);
+    qry->targetList = expandNSItemAttrs(pstate, nsitem, 0, -1);

     /*
      * The grammar allows attaching ORDER BY, LIMIT, and FOR UPDATE to a
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index fe41918..ebbba2d 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -52,7 +52,8 @@
 #include "utils/syscache.h"

 /* Convenience macro for the most common makeNamespaceItem() case */
-#define makeDefaultNSItem(rte)    makeNamespaceItem(rte, true, true, false, true)
+#define makeDefaultNSItem(rte, rti) \
+    makeNamespaceItem(rte, rti, true, true, false, true)

 static void extractRemainingColumns(List *common_colnames,
                                     List *src_colnames, List *src_colvars,
@@ -78,7 +79,7 @@ static Node *transformFromClauseItem(ParseState *pstate, Node *n,
                                      List **namespace);
 static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype,
                                 Var *l_colvar, Var *r_colvar);
-static ParseNamespaceItem *makeNamespaceItem(RangeTblEntry *rte,
+static ParseNamespaceItem *makeNamespaceItem(RangeTblEntry *rte, int rtindex,
                                              bool rel_visible, bool cols_visible,
                                              bool lateral_only, bool lateral_ok);
 static void setNamespaceColumnVisibility(List *namespace, bool cols_visible);
@@ -216,12 +217,15 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
     rte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation,
                                         RowExclusiveLock,
                                         relation->alias, inh, false);
-    pstate->p_target_rangetblentry = rte;

     /* assume new rte is at end */
     rtindex = list_length(pstate->p_rtable);
     Assert(rte == rt_fetch(rtindex, pstate->p_rtable));

+    /* remember the RTE as being the query target */
+    pstate->p_target_rangetblentry = rte;
+    pstate->p_target_rtindex = rtindex;
+
     /*
      * Override addRangeTableEntry's default ACL_SELECT permissions check, and
      * instead mark target table as requiring exactly the specified
@@ -1084,7 +1088,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
         *top_rte = rte;
         *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte));
+        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
         rtr = makeNode(RangeTblRef);
         rtr->rtindex = rtindex;
         return (Node *) rtr;
@@ -1102,7 +1106,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
         *top_rte = rte;
         *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte));
+        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
         rtr = makeNode(RangeTblRef);
         rtr->rtindex = rtindex;
         return (Node *) rtr;
@@ -1120,7 +1124,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
         *top_rte = rte;
         *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte));
+        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
         rtr = makeNode(RangeTblRef);
         rtr->rtindex = rtindex;
         return (Node *) rtr;
@@ -1138,7 +1142,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
         *top_rte = rte;
         *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte));
+        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
         rtr = makeNode(RangeTblRef);
         rtr->rtindex = rtindex;
         return (Node *) rtr;
@@ -1481,6 +1485,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
          */
         *namespace = lappend(my_namespace,
                              makeNamespaceItem(rte,
+                                               j->rtindex,
                                                (j->alias != NULL),
                                                true,
                                                false,
@@ -1617,13 +1622,15 @@ buildMergedJoinVar(ParseState *pstate, JoinType jointype,
  *      Convenience subroutine to construct a ParseNamespaceItem.
  */
 static ParseNamespaceItem *
-makeNamespaceItem(RangeTblEntry *rte, bool rel_visible, bool cols_visible,
+makeNamespaceItem(RangeTblEntry *rte, int rtindex,
+                  bool rel_visible, bool cols_visible,
                   bool lateral_only, bool lateral_ok)
 {
     ParseNamespaceItem *nsitem;

     nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
     nsitem->p_rte = rte;
+    nsitem->p_rtindex = rtindex;
     nsitem->p_rel_visible = rel_visible;
     nsitem->p_cols_visible = cols_visible;
     nsitem->p_lateral_only = lateral_only;
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index eb91da2..25e92de 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -114,8 +114,9 @@ static Node *transformXmlSerialize(ParseState *pstate, XmlSerialize *xs);
 static Node *transformBooleanTest(ParseState *pstate, BooleanTest *b);
 static Node *transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr);
 static Node *transformColumnRef(ParseState *pstate, ColumnRef *cref);
-static Node *transformWholeRowRef(ParseState *pstate, RangeTblEntry *rte,
-                                  int location);
+static Node *transformWholeRowRef(ParseState *pstate,
+                                  ParseNamespaceItem *nsitem,
+                                  int sublevels_up, int location);
 static Node *transformIndirection(ParseState *pstate, A_Indirection *ind);
 static Node *transformTypeCast(ParseState *pstate, TypeCast *tc);
 static Node *transformCollateClause(ParseState *pstate, CollateClause *c);
@@ -510,7 +511,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
     char       *nspname = NULL;
     char       *relname = NULL;
     char       *colname = NULL;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;
     int            levels_up;
     enum
     {
@@ -653,11 +654,11 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                      * PostQUEL-inspired syntax.  The preferred form now is
                      * "rel.*".
                      */
-                    rte = refnameRangeTblEntry(pstate, NULL, colname,
-                                               cref->location,
-                                               &levels_up);
-                    if (rte)
-                        node = transformWholeRowRef(pstate, rte,
+                    nsitem = refnameNamespaceItem(pstate, NULL, colname,
+                                                  cref->location,
+                                                  &levels_up);
+                    if (nsitem)
+                        node = transformWholeRowRef(pstate, nsitem, levels_up,
                                                     cref->location);
                 }
                 break;
@@ -670,11 +671,11 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                 Assert(IsA(field1, String));
                 relname = strVal(field1);

-                /* Locate the referenced RTE */
-                rte = refnameRangeTblEntry(pstate, nspname, relname,
-                                           cref->location,
-                                           &levels_up);
-                if (rte == NULL)
+                /* Locate the referenced nsitem */
+                nsitem = refnameNamespaceItem(pstate, nspname, relname,
+                                              cref->location,
+                                              &levels_up);
+                if (nsitem == NULL)
                 {
                     crerr = CRERR_NO_RTE;
                     break;
@@ -683,20 +684,22 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                 /* Whole-row reference? */
                 if (IsA(field2, A_Star))
                 {
-                    node = transformWholeRowRef(pstate, rte, cref->location);
+                    node = transformWholeRowRef(pstate, nsitem, levels_up,
+                                                cref->location);
                     break;
                 }

                 Assert(IsA(field2, String));
                 colname = strVal(field2);

-                /* Try to identify as a column of the RTE */
-                node = scanRTEForColumn(pstate, rte, colname, cref->location,
-                                        0, NULL);
+                /* Try to identify as a column of the nsitem */
+                node = scanNSItemForColumn(pstate, nsitem, levels_up, colname,
+                                           cref->location);
                 if (node == NULL)
                 {
                     /* Try it as a function call on the whole row */
-                    node = transformWholeRowRef(pstate, rte, cref->location);
+                    node = transformWholeRowRef(pstate, nsitem, levels_up,
+                                                cref->location);
                     node = ParseFuncOrColumn(pstate,
                                              list_make1(makeString(colname)),
                                              list_make1(node),
@@ -718,11 +721,11 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                 Assert(IsA(field2, String));
                 relname = strVal(field2);

-                /* Locate the referenced RTE */
-                rte = refnameRangeTblEntry(pstate, nspname, relname,
-                                           cref->location,
-                                           &levels_up);
-                if (rte == NULL)
+                /* Locate the referenced nsitem */
+                nsitem = refnameNamespaceItem(pstate, nspname, relname,
+                                              cref->location,
+                                              &levels_up);
+                if (nsitem == NULL)
                 {
                     crerr = CRERR_NO_RTE;
                     break;
@@ -731,20 +734,22 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                 /* Whole-row reference? */
                 if (IsA(field3, A_Star))
                 {
-                    node = transformWholeRowRef(pstate, rte, cref->location);
+                    node = transformWholeRowRef(pstate, nsitem, levels_up,
+                                                cref->location);
                     break;
                 }

                 Assert(IsA(field3, String));
                 colname = strVal(field3);

-                /* Try to identify as a column of the RTE */
-                node = scanRTEForColumn(pstate, rte, colname, cref->location,
-                                        0, NULL);
+                /* Try to identify as a column of the nsitem */
+                node = scanNSItemForColumn(pstate, nsitem, levels_up, colname,
+                                           cref->location);
                 if (node == NULL)
                 {
                     /* Try it as a function call on the whole row */
-                    node = transformWholeRowRef(pstate, rte, cref->location);
+                    node = transformWholeRowRef(pstate, nsitem, levels_up,
+                                                cref->location);
                     node = ParseFuncOrColumn(pstate,
                                              list_make1(makeString(colname)),
                                              list_make1(node),
@@ -779,11 +784,11 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                     break;
                 }

-                /* Locate the referenced RTE */
-                rte = refnameRangeTblEntry(pstate, nspname, relname,
-                                           cref->location,
-                                           &levels_up);
-                if (rte == NULL)
+                /* Locate the referenced nsitem */
+                nsitem = refnameNamespaceItem(pstate, nspname, relname,
+                                              cref->location,
+                                              &levels_up);
+                if (nsitem == NULL)
                 {
                     crerr = CRERR_NO_RTE;
                     break;
@@ -792,20 +797,22 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                 /* Whole-row reference? */
                 if (IsA(field4, A_Star))
                 {
-                    node = transformWholeRowRef(pstate, rte, cref->location);
+                    node = transformWholeRowRef(pstate, nsitem, levels_up,
+                                                cref->location);
                     break;
                 }

                 Assert(IsA(field4, String));
                 colname = strVal(field4);

-                /* Try to identify as a column of the RTE */
-                node = scanRTEForColumn(pstate, rte, colname, cref->location,
-                                        0, NULL);
+                /* Try to identify as a column of the nsitem */
+                node = scanNSItemForColumn(pstate, nsitem, levels_up, colname,
+                                           cref->location);
                 if (node == NULL)
                 {
                     /* Try it as a function call on the whole row */
-                    node = transformWholeRowRef(pstate, rte, cref->location);
+                    node = transformWholeRowRef(pstate, nsitem, levels_up,
+                                                cref->location);
                     node = ParseFuncOrColumn(pstate,
                                              list_make1(makeString(colname)),
                                              list_make1(node),
@@ -2648,14 +2655,9 @@ transformBooleanTest(ParseState *pstate, BooleanTest *b)
 static Node *
 transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr)
 {
-    int            sublevels_up;
-
     /* CURRENT OF can only appear at top level of UPDATE/DELETE */
-    Assert(pstate->p_target_rangetblentry != NULL);
-    cexpr->cvarno = RTERangeTablePosn(pstate,
-                                      pstate->p_target_rangetblentry,
-                                      &sublevels_up);
-    Assert(sublevels_up == 0);
+    Assert(pstate->p_target_rtindex > 0);
+    cexpr->cvarno = pstate->p_target_rtindex;

     /*
      * Check to see if the cursor name matches a parameter of type REFCURSOR.
@@ -2703,14 +2705,10 @@ transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr)
  * Construct a whole-row reference to represent the notation "relation.*".
  */
 static Node *
-transformWholeRowRef(ParseState *pstate, RangeTblEntry *rte, int location)
+transformWholeRowRef(ParseState *pstate, ParseNamespaceItem *nsitem,
+                     int sublevels_up, int location)
 {
     Var           *result;
-    int            vnum;
-    int            sublevels_up;
-
-    /* Find the RTE's rangetable location */
-    vnum = RTERangeTablePosn(pstate, rte, &sublevels_up);

     /*
      * Build the appropriate referencing node.  Note that if the RTE is a
@@ -2720,13 +2718,14 @@ transformWholeRowRef(ParseState *pstate, RangeTblEntry *rte, int location)
      * historically.  One argument for it is that "rel" and "rel.*" mean the
      * same thing for composite relations, so why not for scalar functions...
      */
-    result = makeWholeRowVar(rte, vnum, sublevels_up, true);
+    result = makeWholeRowVar(nsitem->p_rte, nsitem->p_rtindex,
+                             sublevels_up, true);

     /* location is not filled in by makeWholeRowVar */
     result->location = location;

     /* mark relation as requiring whole-row SELECT access */
-    markVarForSelectPriv(pstate, result, rte);
+    markVarForSelectPriv(pstate, result, nsitem->p_rte);

     return (Node *) result;
 }
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index d9c6dc1..7efea6e 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -1913,13 +1913,15 @@ ParseComplexProjection(ParseState *pstate, const char *funcname, Node *first_arg
     if (IsA(first_arg, Var) &&
         ((Var *) first_arg)->varattno == InvalidAttrNumber)
     {
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;

-        rte = GetRTEByRangeTablePosn(pstate,
-                                     ((Var *) first_arg)->varno,
-                                     ((Var *) first_arg)->varlevelsup);
+        nsitem = GetNSItemByRangeTablePosn(pstate,
+                                           ((Var *) first_arg)->varno,
+                                           ((Var *) first_arg)->varlevelsup);
         /* Return a Var if funcname matches a column, else NULL */
-        return scanRTEForColumn(pstate, rte, funcname, location, 0, NULL);
+        return scanNSItemForColumn(pstate, nsitem,
+                                   ((Var *) first_arg)->varlevelsup,
+                                   funcname, location);
     }

     /*
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index bc832e7..3936b60 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -181,27 +181,6 @@ pcb_error_callback(void *arg)


 /*
- * make_var
- *        Build a Var node for an attribute identified by RTE and attrno
- */
-Var *
-make_var(ParseState *pstate, RangeTblEntry *rte, int attrno, int location)
-{
-    Var           *result;
-    int            vnum,
-                sublevels_up;
-    Oid            vartypeid;
-    int32        type_mod;
-    Oid            varcollid;
-
-    vnum = RTERangeTablePosn(pstate, rte, &sublevels_up);
-    get_rte_attribute_type(rte, attrno, &vartypeid, &type_mod, &varcollid);
-    result = makeVar(vnum, attrno, vartypeid, type_mod, varcollid, sublevels_up);
-    result->location = location;
-    return result;
-}
-
-/*
  * transformContainerType()
  *        Identify the types involved in a subscripting operation for container
  *
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 47188fc..4888311 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -37,14 +37,37 @@
 #include "utils/syscache.h"
 #include "utils/varlena.h"

+
+/*
+ * Support for fuzzily matching columns.
+ *
+ * This is for building diagnostic messages, where non-exact matching
+ * attributes are suggested to the user.  The struct's fields may be facets of
+ * a particular RTE, or of an entire range table, depending on context.
+ */
+typedef struct
+{
+    int            distance;        /* Weighted distance (lowest so far) */
+    RangeTblEntry *rfirst;        /* RTE of first */
+    AttrNumber    first;            /* Closest attribute so far */
+    RangeTblEntry *rsecond;        /* RTE of second */
+    AttrNumber    second;            /* Second closest attribute so far */
+} FuzzyAttrMatchState;
+
 #define MAX_FUZZY_DISTANCE                3

-static RangeTblEntry *scanNameSpaceForRefname(ParseState *pstate,
-                                              const char *refname, int location);
-static RangeTblEntry *scanNameSpaceForRelid(ParseState *pstate, Oid relid,
-                                            int location);
+
+static ParseNamespaceItem *scanNameSpaceForRefname(ParseState *pstate,
+                                                   const char *refname,
+                                                   int location);
+static ParseNamespaceItem *scanNameSpaceForRelid(ParseState *pstate, Oid relid,
+                                                 int location);
 static void check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem,
                                  int location);
+static int    scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
+                             const char *colname, int location,
+                             int fuzzy_rte_penalty,
+                             FuzzyAttrMatchState *fuzzystate);
 static void markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
                                  int rtindex, AttrNumber col);
 static void expandRelation(Oid relid, Alias *eref,
@@ -61,28 +84,28 @@ static bool isQueryUsingTempRelation_walker(Node *node, void *context);


 /*
- * refnameRangeTblEntry
- *      Given a possibly-qualified refname, look to see if it matches any RTE.
- *      If so, return a pointer to the RangeTblEntry; else return NULL.
+ * refnameNamespaceItem
+ *      Given a possibly-qualified refname, look to see if it matches any visible
+ *      namespace item.  If so, return a pointer to the nsitem; else return NULL.
  *
- *      Optionally get RTE's nesting depth (0 = current) into *sublevels_up.
+ *      Optionally get nsitem's nesting depth (0 = current) into *sublevels_up.
  *      If sublevels_up is NULL, only consider items at the current nesting
  *      level.
  *
- * An unqualified refname (schemaname == NULL) can match any RTE with matching
+ * An unqualified refname (schemaname == NULL) can match any item with matching
  * alias, or matching unqualified relname in the case of alias-less relation
- * RTEs.  It is possible that such a refname matches multiple RTEs in the
+ * items.  It is possible that such a refname matches multiple items in the
  * nearest nesting level that has a match; if so, we report an error via
  * ereport().
  *
- * A qualified refname (schemaname != NULL) can only match a relation RTE
+ * A qualified refname (schemaname != NULL) can only match a relation item
  * that (a) has no alias and (b) is for the same relation identified by
  * schemaname.refname.  In this case we convert schemaname.refname to a
  * relation OID and search by relid, rather than by alias name.  This is
  * peculiar, but it's what SQL says to do.
  */
-RangeTblEntry *
-refnameRangeTblEntry(ParseState *pstate,
+ParseNamespaceItem *
+refnameNamespaceItem(ParseState *pstate,
                      const char *schemaname,
                      const char *refname,
                      int location,
@@ -115,7 +138,7 @@ refnameRangeTblEntry(ParseState *pstate,

     while (pstate != NULL)
     {
-        RangeTblEntry *result;
+        ParseNamespaceItem *result;

         if (OidIsValid(relId))
             result = scanNameSpaceForRelid(pstate, relId, location);
@@ -136,8 +159,8 @@ refnameRangeTblEntry(ParseState *pstate,
 }

 /*
- * Search the query's table namespace for an RTE matching the
- * given unqualified refname.  Return the RTE if a unique match, or NULL
+ * Search the query's table namespace for an item matching the
+ * given unqualified refname.  Return the nsitem if a unique match, or NULL
  * if no match.  Raise error if multiple matches.
  *
  * Note: it might seem that we shouldn't have to worry about the possibility
@@ -152,10 +175,10 @@ refnameRangeTblEntry(ParseState *pstate,
  * this situation, and complain only if there's actually an ambiguous
  * reference to "x".
  */
-static RangeTblEntry *
+static ParseNamespaceItem *
 scanNameSpaceForRefname(ParseState *pstate, const char *refname, int location)
 {
-    RangeTblEntry *result = NULL;
+    ParseNamespaceItem *result = NULL;
     ListCell   *l;

     foreach(l, pstate->p_namespace)
@@ -179,24 +202,24 @@ scanNameSpaceForRefname(ParseState *pstate, const char *refname, int location)
                                 refname),
                          parser_errposition(pstate, location)));
             check_lateral_ref_ok(pstate, nsitem, location);
-            result = rte;
+            result = nsitem;
         }
     }
     return result;
 }

 /*
- * Search the query's table namespace for a relation RTE matching the
- * given relation OID.  Return the RTE if a unique match, or NULL
+ * Search the query's table namespace for a relation item matching the
+ * given relation OID.  Return the nsitem if a unique match, or NULL
  * if no match.  Raise error if multiple matches.
  *
- * See the comments for refnameRangeTblEntry to understand why this
+ * See the comments for refnameNamespaceItem to understand why this
  * acts the way it does.
  */
-static RangeTblEntry *
+static ParseNamespaceItem *
 scanNameSpaceForRelid(ParseState *pstate, Oid relid, int location)
 {
-    RangeTblEntry *result = NULL;
+    ParseNamespaceItem *result = NULL;
     ListCell   *l;

     foreach(l, pstate->p_namespace)
@@ -223,7 +246,7 @@ scanNameSpaceForRelid(ParseState *pstate, Oid relid, int location)
                                 relid),
                          parser_errposition(pstate, location)));
             check_lateral_ref_ok(pstate, nsitem, location);
-            result = rte;
+            result = nsitem;
         }
     }
     return result;
@@ -299,7 +322,7 @@ scanNameSpaceForENR(ParseState *pstate, const char *refname)
  *      See if any RangeTblEntry could possibly match the RangeVar.
  *      If so, return a pointer to the RangeTblEntry; else return NULL.
  *
- * This is different from refnameRangeTblEntry in that it considers every
+ * This is different from refnameNamespaceItem in that it considers every
  * entry in the ParseState's rangetable(s), not only those that are currently
  * visible in the p_namespace list(s).  This behavior is invalid per the SQL
  * spec, and it may give ambiguous results (there might be multiple equally
@@ -431,6 +454,8 @@ checkNameSpaceConflicts(ParseState *pstate, List *namespace1,
  * referencing the target table of an UPDATE or DELETE as a lateral reference
  * in a FROM/USING clause.
  *
+ * Note: the pstate should be the same query level the nsitem was found in.
+ *
  * Convenience subroutine to avoid multiple copies of a rather ugly ereport.
  */
 static void
@@ -456,43 +481,35 @@ check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem,
 }

 /*
- * given an RTE, return RT index (starting with 1) of the entry,
- * and optionally get its nesting depth (0 = current).  If sublevels_up
- * is NULL, only consider rels at the current nesting level.
- * Raises error if RTE not found.
+ * Given an RT index and nesting depth, find the corresponding
+ * ParseNamespaceItem (there must be one).
  */
-int
-RTERangeTablePosn(ParseState *pstate, RangeTblEntry *rte, int *sublevels_up)
+ParseNamespaceItem *
+GetNSItemByRangeTablePosn(ParseState *pstate,
+                          int varno,
+                          int sublevels_up)
 {
-    int            index;
-    ListCell   *l;
-
-    if (sublevels_up)
-        *sublevels_up = 0;
+    ListCell   *lc;

-    while (pstate != NULL)
+    while (sublevels_up-- > 0)
     {
-        index = 1;
-        foreach(l, pstate->p_rtable)
-        {
-            if (rte == (RangeTblEntry *) lfirst(l))
-                return index;
-            index++;
-        }
         pstate = pstate->parentParseState;
-        if (sublevels_up)
-            (*sublevels_up)++;
-        else
-            break;
+        Assert(pstate != NULL);
     }
+    foreach(lc, pstate->p_namespace)
+    {
+        ParseNamespaceItem *nsitem = (ParseNamespaceItem *) lfirst(lc);

-    elog(ERROR, "RTE not found (internal error)");
-    return 0;                    /* keep compiler quiet */
+        if (nsitem->p_rtindex == varno)
+            return nsitem;
+    }
+    elog(ERROR, "nsitem not found (internal error)");
+    return NULL;                /* keep compiler quiet */
 }

 /*
  * Given an RT index and nesting depth, find the corresponding RTE.
- * This is the inverse of RTERangeTablePosn.
+ * (Note that the RTE need not be in the query's namespace.)
  */
 RangeTblEntry *
 GetRTEByRangeTablePosn(ParseState *pstate,
@@ -512,8 +529,7 @@ GetRTEByRangeTablePosn(ParseState *pstate,
  * Fetch the CTE for a CTE-reference RTE.
  *
  * rtelevelsup is the number of query levels above the given pstate that the
- * RTE came from.  Callers that don't have this information readily available
- * may pass -1 instead.
+ * RTE came from.
  */
 CommonTableExpr *
 GetCTEForRTE(ParseState *pstate, RangeTblEntry *rte, int rtelevelsup)
@@ -521,10 +537,6 @@ GetCTEForRTE(ParseState *pstate, RangeTblEntry *rte, int rtelevelsup)
     Index        levelsup;
     ListCell   *lc;

-    /* Determine RTE's levelsup if caller didn't know it */
-    if (rtelevelsup < 0)
-        (void) RTERangeTablePosn(pstate, rte, &rtelevelsup);
-
     Assert(rte->rtekind == RTE_CTE);
     levelsup = rte->ctelevelsup + rtelevelsup;
     while (levelsup-- > 0)
@@ -642,25 +654,94 @@ updateFuzzyAttrMatchState(int fuzzy_rte_penalty,
 }

 /*
+ * scanNSItemForColumn
+ *      Search the column names of a single namespace item for the given name.
+ *      If found, return an appropriate Var node, else return NULL.
+ *      If the name proves ambiguous within this nsitem, raise error.
+ *
+ * Side effect: if we find a match, mark the item's RTE as requiring read
+ * access for the column.
+ */
+Node *
+scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
+                    int sublevels_up, const char *colname, int location)
+{
+    RangeTblEntry *rte = nsitem->p_rte;
+    int            attnum;
+    Var           *var;
+    Oid            vartypeid;
+    int32        vartypmod;
+    Oid            varcollid;
+
+    /*
+     * Scan the RTE's column names (or aliases) for a match.  Complain if
+     * multiple matches.
+     */
+    attnum = scanRTEForColumn(pstate, rte,
+                              colname, location,
+                              0, NULL);
+
+    if (attnum == InvalidAttrNumber)
+        return NULL;            /* Return NULL if no match */
+
+    /* In constraint check, no system column is allowed except tableOid */
+    if (pstate->p_expr_kind == EXPR_KIND_CHECK_CONSTRAINT &&
+        attnum < InvalidAttrNumber && attnum != TableOidAttributeNumber)
+        ereport(ERROR,
+                (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+                 errmsg("system column \"%s\" reference in check constraint is invalid",
+                        colname),
+                 parser_errposition(pstate, location)));
+
+    /* In generated column, no system column is allowed except tableOid */
+    if (pstate->p_expr_kind == EXPR_KIND_GENERATED_COLUMN &&
+        attnum < InvalidAttrNumber && attnum != TableOidAttributeNumber)
+        ereport(ERROR,
+                (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+                 errmsg("cannot use system column \"%s\" in column generation expression",
+                        colname),
+                 parser_errposition(pstate, location)));
+
+    /* Found a valid match, so build a Var */
+    get_rte_attribute_type(rte, attnum,
+                           &vartypeid, &vartypmod, &varcollid);
+    var = makeVar(nsitem->p_rtindex, attnum,
+                  vartypeid, vartypmod, varcollid,
+                  sublevels_up);
+    var->location = location;
+
+    /* Require read access to the column */
+    markVarForSelectPriv(pstate, var, rte);
+
+    return (Node *) var;
+}
+
+/*
  * scanRTEForColumn
  *      Search the column names of a single RTE for the given name.
- *      If found, return an appropriate Var node, else return NULL.
+ *      If found, return the attnum (possibly negative, for a system column);
+ *      else return InvalidAttrNumber.
  *      If the name proves ambiguous within this RTE, raise error.
  *
- * Side effect: if we find a match, mark the RTE as requiring read access
- * for the column.
+ * pstate and location are passed only for error-reporting purposes.
  *
- * Additional side effect: if fuzzystate is non-NULL, check non-system columns
+ * Side effect: if fuzzystate is non-NULL, check non-system columns
  * for an approximate match and update fuzzystate accordingly.
+ *
+ * Note: this is factored out of scanNSItemForColumn because error message
+ * creation may want to check RTEs that are not in the namespace.  To support
+ * that usage, minimize the number of validity checks performed here.  It's
+ * okay to complain about ambiguous-name cases, though, since if we are
+ * working to complain about an invalid name, we've already eliminated that.
  */
-Node *
-scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, const char *colname,
-                 int location, int fuzzy_rte_penalty,
+static int
+scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
+                 const char *colname, int location,
+                 int fuzzy_rte_penalty,
                  FuzzyAttrMatchState *fuzzystate)
 {
-    Node       *result = NULL;
+    int            result = InvalidAttrNumber;
     int            attnum = 0;
-    Var           *var;
     ListCell   *c;

     /*
@@ -673,10 +754,10 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, const char *colname,
      *
      * Should this somehow go wrong and we try to access a dropped column,
      * we'll still catch it by virtue of the checks in
-     * get_rte_attribute_type(), which is called by make_var().  That routine
-     * has to do a cache lookup anyway, so the check there is cheap.  Callers
-     * interested in finding match with shortest distance need to defend
-     * against this directly, though.
+     * get_rte_attribute_type(), which is called by scanNSItemForColumn().
+     * That routine has to do a cache lookup anyway, so the check there is
+     * cheap.  Callers interested in finding match with shortest distance need
+     * to defend against this directly, though.
      */
     foreach(c, rte->eref->colnames)
     {
@@ -691,13 +772,10 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, const char *colname,
                          errmsg("column reference \"%s\" is ambiguous",
                                 colname),
                          parser_errposition(pstate, location)));
-            var = make_var(pstate, rte, attnum, location);
-            /* Require read access to the column */
-            markVarForSelectPriv(pstate, var, rte);
-            result = (Node *) var;
+            result = attnum;
         }

-        /* Updating fuzzy match state, if provided. */
+        /* Update fuzzy match state, if provided. */
         if (fuzzystate != NULL)
             updateFuzzyAttrMatchState(fuzzy_rte_penalty, fuzzystate,
                                       rte, attcolname, colname, attnum);
@@ -720,39 +798,13 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, const char *colname,
     {
         /* quick check to see if name could be a system column */
         attnum = specialAttNum(colname);
-
-        /* In constraint check, no system column is allowed except tableOid */
-        if (pstate->p_expr_kind == EXPR_KIND_CHECK_CONSTRAINT &&
-            attnum < InvalidAttrNumber && attnum != TableOidAttributeNumber)
-            ereport(ERROR,
-                    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
-                     errmsg("system column \"%s\" reference in check constraint is invalid",
-                            colname),
-                     parser_errposition(pstate, location)));
-
-        /*
-         * In generated column, no system column is allowed except tableOid.
-         */
-        if (pstate->p_expr_kind == EXPR_KIND_GENERATED_COLUMN &&
-            attnum < InvalidAttrNumber && attnum != TableOidAttributeNumber)
-            ereport(ERROR,
-                    (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
-                     errmsg("cannot use system column \"%s\" in column generation expression",
-                            colname),
-                     parser_errposition(pstate, location)));
-
         if (attnum != InvalidAttrNumber)
         {
             /* now check to see if column actually is defined */
             if (SearchSysCacheExists2(ATTNUM,
                                       ObjectIdGetDatum(rte->relid),
                                       Int16GetDatum(attnum)))
-            {
-                var = make_var(pstate, rte, attnum, location);
-                /* Require read access to the column */
-                markVarForSelectPriv(pstate, var, rte);
-                result = (Node *) var;
-            }
+                result = attnum;
         }
     }

@@ -771,6 +823,7 @@ colNameToVar(ParseState *pstate, const char *colname, bool localonly,
              int location)
 {
     Node       *result = NULL;
+    int            sublevels_up = 0;
     ParseState *orig_pstate = pstate;

     while (pstate != NULL)
@@ -780,7 +833,6 @@ colNameToVar(ParseState *pstate, const char *colname, bool localonly,
         foreach(l, pstate->p_namespace)
         {
             ParseNamespaceItem *nsitem = (ParseNamespaceItem *) lfirst(l);
-            RangeTblEntry *rte = nsitem->p_rte;
             Node       *newresult;

             /* Ignore table-only items */
@@ -790,9 +842,9 @@ colNameToVar(ParseState *pstate, const char *colname, bool localonly,
             if (nsitem->p_lateral_only && !pstate->p_lateral_active)
                 continue;

-            /* use orig_pstate here to get the right sublevels_up */
-            newresult = scanRTEForColumn(orig_pstate, rte, colname, location,
-                                         0, NULL);
+            /* use orig_pstate here for consistency with other callers */
+            newresult = scanNSItemForColumn(orig_pstate, nsitem, sublevels_up,
+                                            colname, location);

             if (newresult)
             {
@@ -811,6 +863,7 @@ colNameToVar(ParseState *pstate, const char *colname, bool localonly,
             break;                /* found, or don't want to look at parent */

         pstate = pstate->parentParseState;
+        sublevels_up++;
     }

     return result;
@@ -2182,9 +2235,23 @@ addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,
               bool addToJoinList,
               bool addToRelNameSpace, bool addToVarNameSpace)
 {
+    int            rtindex;
+
+    /*
+     * Most callers have just added the RTE to the rangetable, so it's likely
+     * to be the last entry.  Hence, it's a good idea to search the rangetable
+     * back-to-front.
+     */
+    for (rtindex = list_length(pstate->p_rtable); rtindex > 0; rtindex--)
+    {
+        if (rte == rt_fetch(rtindex, pstate->p_rtable))
+            break;
+    }
+    if (rtindex <= 0)
+        elog(ERROR, "RTE not found (internal error)");
+
     if (addToJoinList)
     {
-        int            rtindex = RTERangeTablePosn(pstate, rte, NULL);
         RangeTblRef *rtr = makeNode(RangeTblRef);

         rtr->rtindex = rtindex;
@@ -2196,6 +2263,7 @@ addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,

         nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
         nsitem->p_rte = rte;
+        nsitem->p_rtindex = rtindex;
         nsitem->p_rel_visible = addToRelNameSpace;
         nsitem->p_cols_visible = addToVarNameSpace;
         nsitem->p_lateral_only = false;
@@ -2653,26 +2721,25 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset,
 }

 /*
- * expandRelAttrs -
+ * expandNSItemAttrs -
  *      Workhorse for "*" expansion: produce a list of targetentries
- *      for the attributes of the RTE
+ *      for the attributes of the nsitem
  *
- * As with expandRTE, rtindex/sublevels_up determine the varno/varlevelsup
- * fields of the Vars produced, and location sets their location.
  * pstate->p_next_resno determines the resnos assigned to the TLEs.
  * The referenced columns are marked as requiring SELECT access.
  */
 List *
-expandRelAttrs(ParseState *pstate, RangeTblEntry *rte,
-               int rtindex, int sublevels_up, int location)
+expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
+                  int sublevels_up, int location)
 {
+    RangeTblEntry *rte = nsitem->p_rte;
     List       *names,
                *vars;
     ListCell   *name,
                *var;
     List       *te_list = NIL;

-    expandRTE(rte, rtindex, sublevels_up, location, false,
+    expandRTE(rte, nsitem->p_rtindex, sublevels_up, location, false,
               &names, &vars);

     /*
@@ -3253,7 +3320,6 @@ void
 errorMissingRTE(ParseState *pstate, RangeVar *relation)
 {
     RangeTblEntry *rte;
-    int            sublevels_up;
     const char *badAlias = NULL;

     /*
@@ -3274,11 +3340,17 @@ errorMissingRTE(ParseState *pstate, RangeVar *relation)
      * MySQL-ism "SELECT ... FROM a, b LEFT JOIN c ON (a.x = c.y)".
      */
     if (rte && rte->alias &&
-        strcmp(rte->eref->aliasname, relation->relname) != 0 &&
-        refnameRangeTblEntry(pstate, NULL, rte->eref->aliasname,
-                             relation->location,
-                             &sublevels_up) == rte)
-        badAlias = rte->eref->aliasname;
+        strcmp(rte->eref->aliasname, relation->relname) != 0)
+    {
+        ParseNamespaceItem *nsitem;
+        int            sublevels_up;
+
+        nsitem = refnameNamespaceItem(pstate, NULL, rte->eref->aliasname,
+                                      relation->location,
+                                      &sublevels_up);
+        if (nsitem && nsitem->p_rte == rte)
+            badAlias = rte->eref->aliasname;
+    }

     if (rte)
         ereport(ERROR,
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 30d419e..46fe6c6 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -62,8 +62,9 @@ static List *ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
 static List *ExpandAllTables(ParseState *pstate, int location);
 static List *ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind,
                                    bool make_target_entry, ParseExprKind exprKind);
-static List *ExpandSingleTable(ParseState *pstate, RangeTblEntry *rte,
-                               int location, bool make_target_entry);
+static List *ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem,
+                               int sublevels_up, int location,
+                               bool make_target_entry);
 static List *ExpandRowReference(ParseState *pstate, Node *expr,
                                 bool make_target_entry);
 static int    FigureColnameInternal(Node *node, char **name);
@@ -548,10 +549,13 @@ transformAssignedExpr(ParseState *pstate,
             /*
              * Build a Var for the column to be updated.
              */
-            colVar = (Node *) make_var(pstate,
-                                       pstate->p_target_rangetblentry,
-                                       attrno,
-                                       location);
+            Var           *var;
+
+            var = makeVar(pstate->p_target_rtindex, attrno,
+                          attrtype, attrtypmod, attrcollation, 0);
+            var->location = location;
+
+            colVar = (Node *) var;
         }

         expr = (Expr *)
@@ -1127,7 +1131,7 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
          */
         char       *nspname = NULL;
         char       *relname = NULL;
-        RangeTblEntry *rte = NULL;
+        ParseNamespaceItem *nsitem = NULL;
         int            levels_up;
         enum
         {
@@ -1153,16 +1157,16 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
         {
             case 2:
                 relname = strVal(linitial(fields));
-                rte = refnameRangeTblEntry(pstate, nspname, relname,
-                                           cref->location,
-                                           &levels_up);
+                nsitem = refnameNamespaceItem(pstate, nspname, relname,
+                                              cref->location,
+                                              &levels_up);
                 break;
             case 3:
                 nspname = strVal(linitial(fields));
                 relname = strVal(lsecond(fields));
-                rte = refnameRangeTblEntry(pstate, nspname, relname,
-                                           cref->location,
-                                           &levels_up);
+                nsitem = refnameNamespaceItem(pstate, nspname, relname,
+                                              cref->location,
+                                              &levels_up);
                 break;
             case 4:
                 {
@@ -1178,9 +1182,9 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
                     }
                     nspname = strVal(lsecond(fields));
                     relname = strVal(lthird(fields));
-                    rte = refnameRangeTblEntry(pstate, nspname, relname,
-                                               cref->location,
-                                               &levels_up);
+                    nsitem = refnameNamespaceItem(pstate, nspname, relname,
+                                                  cref->location,
+                                                  &levels_up);
                     break;
                 }
             default:
@@ -1193,17 +1197,19 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
          * bit by passing the RangeTblEntry, not a Var, as the planned
          * translation.  (A single Var wouldn't be strictly correct anyway.
          * This convention allows hooks that really care to know what is
-         * happening.)
+         * happening.  It might be better to pass the nsitem, but we'd have to
+         * promote that struct to a full-fledged Node type so that callees
+         * could identify its type.)
          */
         if (pstate->p_post_columnref_hook != NULL)
         {
             Node       *node;

             node = pstate->p_post_columnref_hook(pstate, cref,
-                                                 (Node *) rte);
+                                                 (Node *) (nsitem ? nsitem->p_rte : NULL));
             if (node != NULL)
             {
-                if (rte != NULL)
+                if (nsitem != NULL)
                     ereport(ERROR,
                             (errcode(ERRCODE_AMBIGUOUS_COLUMN),
                              errmsg("column reference \"%s\" is ambiguous",
@@ -1216,7 +1222,7 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
         /*
          * Throw error if no translation found.
          */
-        if (rte == NULL)
+        if (nsitem == NULL)
         {
             switch (crserr)
             {
@@ -1242,9 +1248,10 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
         }

         /*
-         * OK, expand the RTE into fields.
+         * OK, expand the nsitem into fields.
          */
-        return ExpandSingleTable(pstate, rte, cref->location, make_target_entry);
+        return ExpandSingleTable(pstate, nsitem, levels_up, cref->location,
+                                 make_target_entry);
     }
 }

@@ -1269,7 +1276,6 @@ ExpandAllTables(ParseState *pstate, int location)
     foreach(l, pstate->p_namespace)
     {
         ParseNamespaceItem *nsitem = (ParseNamespaceItem *) lfirst(l);
-        RangeTblEntry *rte = nsitem->p_rte;

         /* Ignore table-only items */
         if (!nsitem->p_cols_visible)
@@ -1280,12 +1286,10 @@ ExpandAllTables(ParseState *pstate, int location)
         found_table = true;

         target = list_concat(target,
-                             expandRelAttrs(pstate,
-                                            rte,
-                                            RTERangeTablePosn(pstate, rte,
-                                                              NULL),
-                                            0,
-                                            location));
+                             expandNSItemAttrs(pstate,
+                                               nsitem,
+                                               0,
+                                               location));
     }

     /*
@@ -1341,26 +1345,21 @@ ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind,
  * The referenced columns are marked as requiring SELECT access.
  */
 static List *
-ExpandSingleTable(ParseState *pstate, RangeTblEntry *rte,
-                  int location, bool make_target_entry)
+ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem,
+                  int sublevels_up, int location, bool make_target_entry)
 {
-    int            sublevels_up;
-    int            rtindex;
-
-    rtindex = RTERangeTablePosn(pstate, rte, &sublevels_up);
-
     if (make_target_entry)
     {
-        /* expandRelAttrs handles permissions marking */
-        return expandRelAttrs(pstate, rte, rtindex, sublevels_up,
-                              location);
+        /* expandNSItemAttrs handles permissions marking */
+        return expandNSItemAttrs(pstate, nsitem, sublevels_up, location);
     }
     else
     {
+        RangeTblEntry *rte = nsitem->p_rte;
         List       *vars;
         ListCell   *l;

-        expandRTE(rte, rtindex, sublevels_up, location, false,
+        expandRTE(rte, nsitem->p_rtindex, sublevels_up, location, false,
                   NULL, &vars);

         /*
@@ -1411,10 +1410,10 @@ ExpandRowReference(ParseState *pstate, Node *expr,
         ((Var *) expr)->varattno == InvalidAttrNumber)
     {
         Var           *var = (Var *) expr;
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;

-        rte = GetRTEByRangeTablePosn(pstate, var->varno, var->varlevelsup);
-        return ExpandSingleTable(pstate, rte, var->location, make_target_entry);
+        nsitem = GetNSItemByRangeTablePosn(pstate, var->varno, var->varlevelsup);
+        return ExpandSingleTable(pstate, nsitem, var->varlevelsup, var->location, make_target_entry);
     }

     /*
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 7c099e7..674acc5 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -134,6 +134,8 @@ typedef Node *(*CoerceParamHook) (ParseState *pstate, Param *param,
  *
  * p_target_rangetblentry: target relation's entry in the rtable list.
  *
+ * p_target_rtindex: target relation's index in the rtable list.
+ *
  * p_is_insert: true to process assignment expressions like INSERT, false
  * to process them like UPDATE.  (Note this can change intra-statement, for
  * cases like INSERT ON CONFLICT UPDATE.)
@@ -185,7 +187,8 @@ struct ParseState
     List       *p_future_ctes;    /* common table exprs not yet in namespace */
     CommonTableExpr *p_parent_cte;    /* this query's containing CTE */
     Relation    p_target_relation;    /* INSERT/UPDATE/DELETE target rel */
-    RangeTblEntry *p_target_rangetblentry;    /* target rel's RTE */
+    RangeTblEntry *p_target_rangetblentry;    /* target rel's RTE, or NULL */
+    int            p_target_rtindex;    /* target rel's RT index, or 0 */
     bool        p_is_insert;    /* process assignment like INSERT not UPDATE */
     List       *p_windowdefs;    /* raw representations of window clauses */
     ParseExprKind p_expr_kind;    /* what kind of expression we're parsing */
@@ -249,6 +252,7 @@ struct ParseState
 typedef struct ParseNamespaceItem
 {
     RangeTblEntry *p_rte;        /* The relation's rangetable entry */
+    int            p_rtindex;        /* The relation's index in the rangetable */
     bool        p_rel_visible;    /* Relation name is visible? */
     bool        p_cols_visible; /* Column names visible as unqualified refs? */
     bool        p_lateral_only; /* Is only visible to LATERAL expressions? */
@@ -272,8 +276,6 @@ extern void setup_parser_errposition_callback(ParseCallbackState *pcbstate,
                                               ParseState *pstate, int location);
 extern void cancel_parser_errposition_callback(ParseCallbackState *pcbstate);

-extern Var *make_var(ParseState *pstate, RangeTblEntry *rte, int attrno,
-                     int location);
 extern Oid    transformContainerType(Oid *containerType, int32 *containerTypmod);

 extern SubscriptingRef *transformContainerSubscripts(ParseState *pstate,
diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
index f7e0781..b09a71e 100644
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -17,45 +17,28 @@
 #include "parser/parse_node.h"


-/*
- * Support for fuzzily matching column.
- *
- * This is for building diagnostic messages, where non-exact matching
- * attributes are suggested to the user.  The struct's fields may be facets of
- * a particular RTE, or of an entire range table, depending on context.
- */
-typedef struct
-{
-    int            distance;        /* Weighted distance (lowest so far) */
-    RangeTblEntry *rfirst;        /* RTE of first */
-    AttrNumber    first;            /* Closest attribute so far */
-    RangeTblEntry *rsecond;        /* RTE of second */
-    AttrNumber    second;            /* Second closest attribute so far */
-} FuzzyAttrMatchState;
-
-
-extern RangeTblEntry *refnameRangeTblEntry(ParseState *pstate,
-                                           const char *schemaname,
-                                           const char *refname,
-                                           int location,
-                                           int *sublevels_up);
+extern ParseNamespaceItem *refnameNamespaceItem(ParseState *pstate,
+                                                const char *schemaname,
+                                                const char *refname,
+                                                int location,
+                                                int *sublevels_up);
 extern CommonTableExpr *scanNameSpaceForCTE(ParseState *pstate,
                                             const char *refname,
                                             Index *ctelevelsup);
 extern bool scanNameSpaceForENR(ParseState *pstate, const char *refname);
 extern void checkNameSpaceConflicts(ParseState *pstate, List *namespace1,
                                     List *namespace2);
-extern int    RTERangeTablePosn(ParseState *pstate,
-                              RangeTblEntry *rte,
-                              int *sublevels_up);
+extern ParseNamespaceItem *GetNSItemByRangeTablePosn(ParseState *pstate,
+                                                     int varno,
+                                                     int sublevels_up);
 extern RangeTblEntry *GetRTEByRangeTablePosn(ParseState *pstate,
                                              int varno,
                                              int sublevels_up);
 extern CommonTableExpr *GetCTEForRTE(ParseState *pstate, RangeTblEntry *rte,
                                      int rtelevelsup);
-extern Node *scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
-                              const char *colname, int location,
-                              int fuzzy_rte_penalty, FuzzyAttrMatchState *fuzzystate);
+extern Node *scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
+                                 int sublevels_up, const char *colname,
+                                 int location);
 extern Node *colNameToVar(ParseState *pstate, const char *colname, bool localonly,
                           int location);
 extern void markVarForSelectPriv(ParseState *pstate, Var *var,
@@ -122,8 +105,8 @@ extern void errorMissingColumn(ParseState *pstate,
 extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                       int location, bool include_dropped,
                       List **colnames, List **colvars);
-extern List *expandRelAttrs(ParseState *pstate, RangeTblEntry *rte,
-                            int rtindex, int sublevels_up, int location);
+extern List *expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
+                               int sublevels_up, int location);
 extern int    attnameAttNum(Relation rd, const char *attname, bool sysColOK);
 extern const NameData *attnumAttName(Relation rd, int attid);
 extern Oid    attnumTypeId(Relation rd, int attid);

Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Robert Haas
Дата:
On Fri, Dec 20, 2019 at 11:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The alternatives that seem plausible at this point are
>
> (1) Create some sort of wrapper node indicating "the contents of this
> expression might be replaced by NULL".  This is basically what the
> planner's PlaceHolderVars do, so maybe we'd just be talking about
> introducing those at some earlier stage.
>
> (2) Explicitly mark Vars as being nullable by some outer join.  I think
> we could probably get this down to one additional integer field in
> struct Var, so it wouldn't be too much of a penalty.
>
> The wrapper approach is more general since you can wrap something
> that's not necessarily a plain Var; but it's also bulkier and so
> probably a bit less efficient.  I'm not sure which idea I like better.

I'm not sure which is better, either, although I would like to note in
passing that the name PlaceHolderVar seems to me to be confusing and
terrible. It took me years to understand it, and I've never been
totally sure that I actually do. Why is it not called
MightBeNullWrapper or something?

If you chose to track it in the Var, maybe you could do better than to
track whether it might have gone to NULL. For example, perhaps you
could track the set of baserels that are syntactically below the Var
location and have the Var on the nullable side of a join, rather than
just have a Boolean that indicates whether there are any. I don't know
whether the additional effort would be worth the cost of maintaining
the information, but it seems like it might be.

> With either approach, we could either make parse analysis inject the
> nullability markings, or wait to do it in the planner.  On a purely
> abstract system structural level, I like the former better: it is
> exactly the province of parse analysis to decide what are the semantics
> of what the user typed, and surely what a Var means is part of that.
> OTOH, if we do it that way, the planner potentially has to rearrange the
> markings after it does join strength reduction; so maybe it's best to
> just wait till after that planning phase to address this at all.
>
> Any thoughts about that?

Generally, I like the idea of driving this off the parse tree, because
it seems to me that, ultimately, whether a Var is *potentially*
nullable or not depends on the query as provided by the user. And, if
we replan the query, these determinations don't change, at least as
long as they are only driven by the query syntax and not, say,
attisnull or opclass details. It would be nice not to redo the work
unnecessarily. However, that seems to require some way of segregating
the information we derive as a preliminary and syntactical judgement
from subsequent inferences made during query planning, because the
latter CAN change during replanning.

It might be useful 'Relids' with each Var rather than just 'bool'. In
other words, based on where the reference to the Var is in the
original query text, figure out the set of joins where (1) the Var is
syntactically above the join and (2) on the nullable side, and then
put the relations on the other sides of those joins into the Relids.
Then if you later determine that A LEFT JOIN B actually can't make
anything go to null, you can just ignore the presence of A in this set
for the rest of planning. I feel like this kind of idea might have
other applications too, although I admit that it also has a cost.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Dec 20, 2019 at 11:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The alternatives that seem plausible at this point are
>> ...
>> (2) Explicitly mark Vars as being nullable by some outer join.  I think
>> we could probably get this down to one additional integer field in
>> struct Var, so it wouldn't be too much of a penalty.

> It might be useful 'Relids' with each Var rather than just 'bool'. In
> other words, based on where the reference to the Var is in the
> original query text, figure out the set of joins where (1) the Var is
> syntactically above the join and (2) on the nullable side, and then
> put the relations on the other sides of those joins into the Relids.
> Then if you later determine that A LEFT JOIN B actually can't make
> anything go to null, you can just ignore the presence of A in this set
> for the rest of planning. I feel like this kind of idea might have
> other applications too, although I admit that it also has a cost.

Yeah, a bitmapset might be a better idea than just recording the topmost
relevant join's relid.  But it's also more expensive, and I think we ought
to keep the representation of Vars as cheap as possible.  (On the third
hand, an empty BMS is cheap, while if the alternative to a non-empty BMS
is to put a separate wrapper node around the Var, that's hardly better.)

The key advantage of a BMS, really, is that it dodges the issue of needing
to re-mark Vars when you re-order two outer joins using the outer join
identities.  You really don't want that to result in having to consider
Vars above the two joins to be different depending on the order you chose
for the OJs, because that'd enormously complicate considering both sorts
of Paths at the same time.  The rough idea I'd had about coping with that
issue with just a single relid is that maybe it doesn't matter --- maybe
we can always mark Vars according to the *syntactically* highest nulling
OJ, regardless of the actual join order.  But I'm not totally sure that
can work.

In any case, what the planner likes to work with is sets of baserel
relids covered by a join, not the relid(s) of the join RTEs themselves.
So maybe there's going to be a conversion step required anyhow.

            regards, tom lane



Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Andres Freund
Дата:
Hi,

On 2019-12-20 11:12:53 -0500, Tom Lane wrote:
> (2) Explicitly mark Vars as being nullable by some outer join.  I think
> we could probably get this down to one additional integer field in
> struct Var, so it wouldn't be too much of a penalty.

I've for a while wished that we could, e.g. so execution can use faster
tuple deforming code, infer nullability of columns above the scan
level. Right now there's no realistic way ExecTypeFromTL() can figure
that out, for upper query nodes. If we were to introduce something like
the field you suggest, it'd be darn near trivial.

OTOH, I'd really at some point like to start moving TupleDesc
computations to the planner - they're quite expensive, and we do them
over and over again. And that would not necessarily need a convenient
execution time representation anymore. But I don't think moving
tupledesc computation into the planner is a small rearrangement...


Would we want to have only boolean state, or more information (e.g. not
null, maybe null, is null)?


Greetings,

Andres Freund



Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
I wrote:
> Anyway, I had started to work on getting parse analysis to label
> outer-join-nullable Vars properly, and soon decided that no matter how
> we do it, there's not enough information available at the point where
> parse analysis makes a Var.  The referenced RTE is not, in itself,
> enough info, and I don't think we want to decorate RTEs with more info
> that's only needed during parse analysis.  What would be saner is to add
> any extra info to the ParseNamespaceItem structs.

Here is a further step on this journey.  It's still just parser
refactoring, and doesn't (AFAICT) result in any change in generated
parse trees, but it seems worth posting and committing separately.

The two key ideas here are:

1. Integrate ParseNamespaceItems a bit further into the parser's
relevant APIs.  In particular, the addRangeTableEntryXXX functions
no longer return just a bare RTE, but a ParseNamespaceItem wrapper
for it.  This gets rid of a number of kluges we had for finding out
the RT index of the new RTE, since that's now carried along in the
nsitem --- we no longer need fragile assumptions about how the new
RTE is still the last one in the rangetable, at some point rather
distant from where it was actually appended to the list.

Most of the callers of addRangeTableEntryXXX functions just turn
around and pass the result to addRTEtoQuery, which I've renamed
to addNSItemtoQuery; it doesn't gin up a new nsitem anymore but
just installs the one it's passed.  It's perhaps a bit inconsistent
that I renamed that function but not addRangeTableEntryXXX.
I considered making those addNamespaceItemXXX, but desisted on the
perhaps-thin grounds that they don't link the new nsitem into the
parse state, only the RTE.  This could be argued of course.

2. Add per-column information to the ParseNamespaceItems.  As of
this patch, the useful part of that is column type/typmod/collation
info which can be used to generate Vars referencing this RTE.
I envision that the next step will involve generating the Vars'
identity (varno/varattno columns) from that as well, and this
patch includes logic to set up some associated per-column fields.
But those are not actually getting fed into the Vars quite yet.
(The step after that will be to add outer-join-nullability info.)

But independently of those future improvements, this patch is
a win because it allows carrying forward column-type info that's
known at the time we do addRangeTableEntryXXX, and using that
when we make a Var, instead of having to do the rather expensive
computations involved in expandRTE() or get_rte_attribute_type().
get_rte_attribute_type() is indeed gone altogether, and while
expandRTE() is still needed, it's not used in any performance-critical
parse analysis code paths.

On a complex-query test case that I've used before [1], microbenchmarking
just raw parsing plus parse analysis shows a full 20% speedup over HEAD,
which I think can mostly be attributed to getting rid of the syscache
lookups that get_rte_attribute_type() did for Vars referencing base
relations.  The total impact over a complete query execution cycle
is a lot less of course.  Still, it's pretty clearly a performance win,
and to my mind the code is also cleaner --- this is paying down some
technical debt from when we bolted JOIN syntax onto pre-existing
parsing code.

Barring objections, I plan to commit this fairly soon and get onto the
next step, which will start to have ramifications outside the parser.

            regards, tom lane

[1] https://www.postgresql.org/message-id/6970.1545327857%40sss.pgh.pa.us

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 8b68fb7..07533f6 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2531,7 +2531,7 @@ AddRelationNewConstraints(Relation rel,
     TupleConstr *oldconstr;
     int            numoldchecks;
     ParseState *pstate;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;
     int            numchecks;
     List       *checknames;
     ListCell   *cell;
@@ -2554,13 +2554,13 @@ AddRelationNewConstraints(Relation rel,
      */
     pstate = make_parsestate(NULL);
     pstate->p_sourcetext = queryString;
-    rte = addRangeTableEntryForRelation(pstate,
-                                        rel,
-                                        AccessShareLock,
-                                        NULL,
-                                        false,
-                                        true);
-    addRTEtoQuery(pstate, rte, true, true, true);
+    nsitem = addRangeTableEntryForRelation(pstate,
+                                           rel,
+                                           AccessShareLock,
+                                           NULL,
+                                           false,
+                                           true);
+    addNSItemtoQuery(pstate, nsitem, true, true, true);

     /*
      * Process column default expressions.
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 42a147b..04d5bde 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -882,6 +882,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
     if (stmt->relation)
     {
         LOCKMODE    lockmode = is_from ? RowExclusiveLock : AccessShareLock;
+        ParseNamespaceItem *nsitem;
         RangeTblEntry *rte;
         TupleDesc    tupDesc;
         List       *attnums;
@@ -894,14 +895,15 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,

         relid = RelationGetRelid(rel);

-        rte = addRangeTableEntryForRelation(pstate, rel, lockmode,
-                                            NULL, false, false);
+        nsitem = addRangeTableEntryForRelation(pstate, rel, lockmode,
+                                               NULL, false, false);
+        rte = nsitem->p_rte;
         rte->requiredPerms = (is_from ? ACL_INSERT : ACL_SELECT);

         if (stmt->whereClause)
         {
-            /* add rte to column namespace  */
-            addRTEtoQuery(pstate, rte, false, true, true);
+            /* add nsitem to column namespace  */
+            addNSItemtoQuery(pstate, nsitem, false, true, true);

             /* Transform the raw expression tree */
             whereClause = transformExpr(pstate, stmt->whereClause, EXPR_KIND_COPY_WHERE);
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 36093dc..1e538a5 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -568,9 +568,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
             qual_expr = stringToNode(qual_value);

             /* Add this rel to the parsestate's rangetable, for dependencies */
-            addRangeTableEntryForRelation(qual_pstate, rel,
-                                          AccessShareLock,
-                                          NULL, false, false);
+            (void) addRangeTableEntryForRelation(qual_pstate, rel,
+                                                 AccessShareLock,
+                                                 NULL, false, false);

             qual_parse_rtable = qual_pstate->p_rtable;
             free_parsestate(qual_pstate);
@@ -592,9 +592,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
             with_check_qual = stringToNode(with_check_value);

             /* Add this rel to the parsestate's rangetable, for dependencies */
-            addRangeTableEntryForRelation(with_check_pstate, rel,
-                                          AccessShareLock,
-                                          NULL, false, false);
+            (void) addRangeTableEntryForRelation(with_check_pstate, rel,
+                                                 AccessShareLock,
+                                                 NULL, false, false);

             with_check_parse_rtable = with_check_pstate->p_rtable;
             free_parsestate(with_check_pstate);
@@ -699,7 +699,7 @@ CreatePolicy(CreatePolicyStmt *stmt)
     ArrayType  *role_ids;
     ParseState *qual_pstate;
     ParseState *with_check_pstate;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;
     Node       *qual;
     Node       *with_check_qual;
     ScanKeyData skey[2];
@@ -755,16 +755,16 @@ CreatePolicy(CreatePolicyStmt *stmt)
     target_table = relation_open(table_id, NoLock);

     /* Add for the regular security quals */
-    rte = addRangeTableEntryForRelation(qual_pstate, target_table,
-                                        AccessShareLock,
-                                        NULL, false, false);
-    addRTEtoQuery(qual_pstate, rte, false, true, true);
+    nsitem = addRangeTableEntryForRelation(qual_pstate, target_table,
+                                           AccessShareLock,
+                                           NULL, false, false);
+    addNSItemtoQuery(qual_pstate, nsitem, false, true, true);

     /* Add for the with-check quals */
-    rte = addRangeTableEntryForRelation(with_check_pstate, target_table,
-                                        AccessShareLock,
-                                        NULL, false, false);
-    addRTEtoQuery(with_check_pstate, rte, false, true, true);
+    nsitem = addRangeTableEntryForRelation(with_check_pstate, target_table,
+                                           AccessShareLock,
+                                           NULL, false, false);
+    addNSItemtoQuery(with_check_pstate, nsitem, false, true, true);

     qual = transformWhereClause(qual_pstate,
                                 copyObject(stmt->qual),
@@ -933,14 +933,14 @@ AlterPolicy(AlterPolicyStmt *stmt)
     /* Parse the using policy clause */
     if (stmt->qual)
     {
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;
         ParseState *qual_pstate = make_parsestate(NULL);

-        rte = addRangeTableEntryForRelation(qual_pstate, target_table,
-                                            AccessShareLock,
-                                            NULL, false, false);
+        nsitem = addRangeTableEntryForRelation(qual_pstate, target_table,
+                                               AccessShareLock,
+                                               NULL, false, false);

-        addRTEtoQuery(qual_pstate, rte, false, true, true);
+        addNSItemtoQuery(qual_pstate, nsitem, false, true, true);

         qual = transformWhereClause(qual_pstate, copyObject(stmt->qual),
                                     EXPR_KIND_POLICY,
@@ -956,14 +956,14 @@ AlterPolicy(AlterPolicyStmt *stmt)
     /* Parse the with-check policy clause */
     if (stmt->with_check)
     {
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;
         ParseState *with_check_pstate = make_parsestate(NULL);

-        rte = addRangeTableEntryForRelation(with_check_pstate, target_table,
-                                            AccessShareLock,
-                                            NULL, false, false);
+        nsitem = addRangeTableEntryForRelation(with_check_pstate, target_table,
+                                               AccessShareLock,
+                                               NULL, false, false);

-        addRTEtoQuery(with_check_pstate, rte, false, true, true);
+        addNSItemtoQuery(with_check_pstate, nsitem, false, true, true);

         with_check_qual = transformWhereClause(with_check_pstate,
                                                copyObject(stmt->with_check),
@@ -1107,9 +1107,9 @@ AlterPolicy(AlterPolicyStmt *stmt)
             qual = stringToNode(qual_value);

             /* Add this rel to the parsestate's rangetable, for dependencies */
-            addRangeTableEntryForRelation(qual_pstate, target_table,
-                                          AccessShareLock,
-                                          NULL, false, false);
+            (void) addRangeTableEntryForRelation(qual_pstate, target_table,
+                                                 AccessShareLock,
+                                                 NULL, false, false);

             qual_parse_rtable = qual_pstate->p_rtable;
             free_parsestate(qual_pstate);
@@ -1149,9 +1149,10 @@ AlterPolicy(AlterPolicyStmt *stmt)
             with_check_qual = stringToNode(with_check_value);

             /* Add this rel to the parsestate's rangetable, for dependencies */
-            addRangeTableEntryForRelation(with_check_pstate, target_table,
-                                          AccessShareLock,
-                                          NULL, false, false);
+            (void) addRangeTableEntryForRelation(with_check_pstate,
+                                                 target_table,
+                                                 AccessShareLock,
+                                                 NULL, false, false);

             with_check_parse_rtable = with_check_pstate->p_rtable;
             free_parsestate(with_check_pstate);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5b882f8..e21cdd7 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -918,7 +918,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
                     defaultPartOid;
         Relation    parent,
                     defaultRel = NULL;
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;

         /* Already have strong enough lock on the parent */
         parent = table_open(parentId, NoLock);
@@ -962,13 +962,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
         pstate->p_sourcetext = queryString;

         /*
-         * Add an RTE containing this relation, so that transformExpr called
-         * on partition bound expressions is able to report errors using a
-         * proper context.
+         * Add an nsitem containing this relation, so that transformExpr
+         * called on partition bound expressions is able to report errors
+         * using a proper context.
          */
-        rte = addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
-                                            NULL, false, false);
-        addRTEtoQuery(pstate, rte, false, true, true);
+        nsitem = addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
+                                               NULL, false, false);
+        addNSItemtoQuery(pstate, nsitem, false, true, true);
+
         bound = transformPartitionBound(pstate, parent, stmt->partbound);

         /*
@@ -14970,7 +14971,7 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy)
 {
     PartitionSpec *newspec;
     ParseState *pstate;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;
     ListCell   *l;

     newspec = makeNode(PartitionSpec);
@@ -15004,9 +15005,9 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy)
      * rangetable entry.  We need a ParseState for transformExpr.
      */
     pstate = make_parsestate(NULL);
-    rte = addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
-                                        NULL, false, true);
-    addRTEtoQuery(pstate, rte, true, true, true);
+    nsitem = addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
+                                           NULL, false, true);
+    addNSItemtoQuery(pstate, nsitem, true, true, true);

     /* take care of any partition expressions */
     foreach(l, partspec->partParams)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 36093a2..18eda58 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -565,7 +565,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
     if (!whenClause && stmt->whenClause)
     {
         ParseState *pstate;
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;
         List       *varList;
         ListCell   *lc;

@@ -574,20 +574,20 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
         pstate->p_sourcetext = queryString;

         /*
-         * Set up RTEs for OLD and NEW references.
+         * Set up nsitems for OLD and NEW references.
          *
          * 'OLD' must always have varno equal to 1 and 'NEW' equal to 2.
          */
-        rte = addRangeTableEntryForRelation(pstate, rel,
-                                            AccessShareLock,
-                                            makeAlias("old", NIL),
-                                            false, false);
-        addRTEtoQuery(pstate, rte, false, true, true);
-        rte = addRangeTableEntryForRelation(pstate, rel,
-                                            AccessShareLock,
-                                            makeAlias("new", NIL),
-                                            false, false);
-        addRTEtoQuery(pstate, rte, false, true, true);
+        nsitem = addRangeTableEntryForRelation(pstate, rel,
+                                               AccessShareLock,
+                                               makeAlias("old", NIL),
+                                               false, false);
+        addNSItemtoQuery(pstate, nsitem, false, true, true);
+        nsitem = addRangeTableEntryForRelation(pstate, rel,
+                                               AccessShareLock,
+                                               makeAlias("new", NIL),
+                                               false, false);
+        addNSItemtoQuery(pstate, nsitem, false, true, true);

         /* Transform expression.  Copy to be sure we don't modify original */
         whenClause = transformWhereClause(pstate,
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 9b51480..5fc02b0 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -341,6 +341,7 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse)
 {
     Relation    viewRel;
     List       *new_rt;
+    ParseNamespaceItem *nsitem;
     RangeTblEntry *rt_entry1,
                *rt_entry2;
     ParseState *pstate;
@@ -365,14 +366,17 @@ UpdateRangeTableOfViewParse(Oid viewOid, Query *viewParse)
      * Create the 2 new range table entries and form the new range table...
      * OLD first, then NEW....
      */
-    rt_entry1 = addRangeTableEntryForRelation(pstate, viewRel,
-                                              AccessShareLock,
-                                              makeAlias("old", NIL),
-                                              false, false);
-    rt_entry2 = addRangeTableEntryForRelation(pstate, viewRel,
-                                              AccessShareLock,
-                                              makeAlias("new", NIL),
-                                              false, false);
+    nsitem = addRangeTableEntryForRelation(pstate, viewRel,
+                                           AccessShareLock,
+                                           makeAlias("old", NIL),
+                                           false, false);
+    rt_entry1 = nsitem->p_rte;
+    nsitem = addRangeTableEntryForRelation(pstate, viewRel,
+                                           AccessShareLock,
+                                           makeAlias("new", NIL),
+                                           false, false);
+    rt_entry2 = nsitem->p_rte;
+
     /* Must override addRangeTableEntry's default access-check flags */
     rt_entry1->requiredPerms = 0;
     rt_entry2->requiredPerms = 0;
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 48b62a5..615309a 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1217,6 +1217,7 @@ convert_ANY_sublink_to_join(PlannerInfo *root, SubLink *sublink,
     Query       *subselect = (Query *) sublink->subselect;
     Relids        upper_varnos;
     int            rtindex;
+    ParseNamespaceItem *nsitem;
     RangeTblEntry *rte;
     RangeTblRef *rtr;
     List       *subquery_vars;
@@ -1264,11 +1265,12 @@ convert_ANY_sublink_to_join(PlannerInfo *root, SubLink *sublink,
      * below). Therefore this is a lot easier than what pull_up_subqueries has
      * to go through.
      */
-    rte = addRangeTableEntryForSubquery(pstate,
-                                        subselect,
-                                        makeAlias("ANY_subquery", NIL),
-                                        false,
-                                        false);
+    nsitem = addRangeTableEntryForSubquery(pstate,
+                                           subselect,
+                                           makeAlias("ANY_subquery", NIL),
+                                           false,
+                                           false);
+    rte = nsitem->p_rte;
     parse->rtable = lappend(parse->rtable, rte);
     rtindex = list_length(parse->rtable);

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0656279..2057bc4 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -415,9 +415,8 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
                                          stmt->relation->inh,
                                          true,
                                          ACL_DELETE);
-
-    /* grab the namespace item made by setTargetTable */
-    nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace);
+    nsitem = pstate->p_target_nsitem;
+    Assert(nsitem->p_rtindex == qry->resultRelation);

     /* there's no DISTINCT in DELETE */
     qry->distinctClause = NIL;
@@ -476,8 +475,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
     List       *sub_namespace;
     List       *icolumns;
     List       *attrnos;
+    ParseNamespaceItem *nsitem;
     RangeTblEntry *rte;
-    RangeTblRef *rtr;
     ListCell   *icols;
     ListCell   *attnos;
     ListCell   *lc;
@@ -613,16 +612,12 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
          * Make the source be a subquery in the INSERT's rangetable, and add
          * it to the INSERT's joinlist.
          */
-        rte = addRangeTableEntryForSubquery(pstate,
-                                            selectQuery,
-                                            makeAlias("*SELECT*", NIL),
-                                            false,
-                                            false);
-        rtr = makeNode(RangeTblRef);
-        /* assume new rte is at end */
-        rtr->rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtr->rtindex, pstate->p_rtable));
-        pstate->p_joinlist = lappend(pstate->p_joinlist, rtr);
+        nsitem = addRangeTableEntryForSubquery(pstate,
+                                               selectQuery,
+                                               makeAlias("*SELECT*", NIL),
+                                               false,
+                                               false);
+        addNSItemtoQuery(pstate, nsitem, true, false, false);

         /*----------
          * Generate an expression list for the INSERT that selects all the
@@ -652,7 +647,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
                 expr = tle->expr;
             else
             {
-                Var           *var = makeVarFromTargetEntry(rtr->rtindex, tle);
+                Var           *var = makeVarFromTargetEntry(nsitem->p_rtindex, tle);

                 var->location = exprLocation((Node *) tle->expr);
                 expr = (Expr *) var;
@@ -774,19 +769,15 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
         /*
          * Generate the VALUES RTE
          */
-        rte = addRangeTableEntryForValues(pstate, exprsLists,
-                                          coltypes, coltypmods, colcollations,
-                                          NULL, lateral, true);
-        rtr = makeNode(RangeTblRef);
-        /* assume new rte is at end */
-        rtr->rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtr->rtindex, pstate->p_rtable));
-        pstate->p_joinlist = lappend(pstate->p_joinlist, rtr);
+        nsitem = addRangeTableEntryForValues(pstate, exprsLists,
+                                             coltypes, coltypmods, colcollations,
+                                             NULL, lateral, true);
+        addNSItemtoQuery(pstate, nsitem, true, false, false);

         /*
          * Generate list of Vars referencing the RTE
          */
-        expandRTE(rte, rtr->rtindex, 0, -1, false, NULL, &exprList);
+        exprList = expandNSItemVars(nsitem, 0, -1, NULL);

         /*
          * Re-apply any indirection on the target column specs to the Vars
@@ -829,7 +820,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
      * Generate query's target list using the computed list of expressions.
      * Also, mark all the target columns as needing insert permissions.
      */
-    rte = pstate->p_target_rangetblentry;
+    rte = pstate->p_target_nsitem->p_rte;
     qry->targetList = NIL;
     Assert(list_length(exprList) <= list_length(icolumns));
     forthree(lc, exprList, icols, icolumns, attnos, attrnos)
@@ -863,8 +854,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
     if (stmt->returningList)
     {
         pstate->p_namespace = NIL;
-        addRTEtoQuery(pstate, pstate->p_target_rangetblentry,
-                      false, true, true);
+        addNSItemtoQuery(pstate, pstate->p_target_nsitem,
+                         false, true, true);
         qry->returningList = transformReturningList(pstate,
                                                     stmt->returningList);
     }
@@ -999,7 +990,6 @@ transformOnConflictClause(ParseState *pstate,
     Oid            arbiterConstraint;
     List       *onConflictSet = NIL;
     Node       *onConflictWhere = NULL;
-    RangeTblEntry *exclRte = NULL;
     int            exclRelIndex = 0;
     List       *exclRelTlist = NIL;
     OnConflictExpr *result;
@@ -1012,6 +1002,8 @@ transformOnConflictClause(ParseState *pstate,
     if (onConflictClause->action == ONCONFLICT_UPDATE)
     {
         Relation    targetrel = pstate->p_target_relation;
+        ParseNamespaceItem *exclNSItem;
+        RangeTblEntry *exclRte;

         /*
          * All INSERT expressions have been parsed, get ready for potentially
@@ -1025,17 +1017,18 @@ transformOnConflictClause(ParseState *pstate,
          * relation, and no permission checks are required on it.  (We'll
          * check the actual target relation, instead.)
          */
-        exclRte = addRangeTableEntryForRelation(pstate,
-                                                targetrel,
-                                                RowExclusiveLock,
-                                                makeAlias("excluded", NIL),
-                                                false, false);
+        exclNSItem = addRangeTableEntryForRelation(pstate,
+                                                   targetrel,
+                                                   RowExclusiveLock,
+                                                   makeAlias("excluded", NIL),
+                                                   false, false);
+        exclRte = exclNSItem->p_rte;
+        exclRelIndex = exclNSItem->p_rtindex;
+
         exclRte->relkind = RELKIND_COMPOSITE_TYPE;
         exclRte->requiredPerms = 0;
         /* other permissions fields in exclRte are already empty */

-        exclRelIndex = list_length(pstate->p_rtable);
-
         /* Create EXCLUDED rel's targetlist for use by EXPLAIN */
         exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
                                                          exclRelIndex);
@@ -1044,9 +1037,9 @@ transformOnConflictClause(ParseState *pstate,
          * Add EXCLUDED and the target RTE to the namespace, so that they can
          * be used in the UPDATE subexpressions.
          */
-        addRTEtoQuery(pstate, exclRte, false, true, true);
-        addRTEtoQuery(pstate, pstate->p_target_rangetblentry,
-                      false, true, true);
+        addNSItemtoQuery(pstate, exclNSItem, false, true, true);
+        addNSItemtoQuery(pstate, pstate->p_target_nsitem,
+                         false, true, true);

         /*
          * Now transform the UPDATE subexpressions.
@@ -1343,7 +1336,6 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
     List      **colexprs = NULL;
     int            sublist_length = -1;
     bool        lateral = false;
-    RangeTblEntry *rte;
     ParseNamespaceItem *nsitem;
     ListCell   *lc;
     ListCell   *lc2;
@@ -1511,14 +1503,10 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
     /*
      * Generate the VALUES RTE
      */
-    rte = addRangeTableEntryForValues(pstate, exprsLists,
-                                      coltypes, coltypmods, colcollations,
-                                      NULL, lateral, true);
-    addRTEtoQuery(pstate, rte, true, true, true);
-
-    /* grab the namespace item made by addRTEtoQuery */
-    nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace);
-    Assert(rte == nsitem->p_rte);
+    nsitem = addRangeTableEntryForValues(pstate, exprsLists,
+                                         coltypes, coltypmods, colcollations,
+                                         NULL, lateral, true);
+    addNSItemtoQuery(pstate, nsitem, true, true, true);

     /*
      * Generate a targetlist as though expanding "*"
@@ -1593,7 +1581,9 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
                *targetnames,
                *sv_namespace;
     int            sv_rtable_length;
-    RangeTblEntry *jrte;
+    ParseNamespaceItem *jnsitem;
+    ParseNamespaceColumn *sortnscolumns;
+    int            sortcolindex;
     int            tllen;

     qry->commandType = CMD_SELECT;
@@ -1686,6 +1676,9 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
     qry->targetList = NIL;
     targetvars = NIL;
     targetnames = NIL;
+    sortnscolumns = (ParseNamespaceColumn *)
+        palloc0(list_length(sostmt->colTypes) * sizeof(ParseNamespaceColumn));
+    sortcolindex = 0;

     forfour(lct, sostmt->colTypes,
             lcm, sostmt->colTypmods,
@@ -1716,6 +1709,14 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
         qry->targetList = lappend(qry->targetList, tle);
         targetvars = lappend(targetvars, var);
         targetnames = lappend(targetnames, makeString(colName));
+        sortnscolumns[sortcolindex].p_varno = leftmostRTI;
+        sortnscolumns[sortcolindex].p_varattno = lefttle->resno;
+        sortnscolumns[sortcolindex].p_vartype = colType;
+        sortnscolumns[sortcolindex].p_vartypmod = colTypmod;
+        sortnscolumns[sortcolindex].p_varcollid = colCollation;
+        sortnscolumns[sortcolindex].p_varnosyn = leftmostRTI;
+        sortnscolumns[sortcolindex].p_varattnosyn = lefttle->resno;
+        sortcolindex++;
     }

     /*
@@ -1730,18 +1731,19 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
      */
     sv_rtable_length = list_length(pstate->p_rtable);

-    jrte = addRangeTableEntryForJoin(pstate,
-                                     targetnames,
-                                     JOIN_INNER,
-                                     targetvars,
-                                     NULL,
-                                     false);
+    jnsitem = addRangeTableEntryForJoin(pstate,
+                                        targetnames,
+                                        sortnscolumns,
+                                        JOIN_INNER,
+                                        targetvars,
+                                        NULL,
+                                        false);

     sv_namespace = pstate->p_namespace;
     pstate->p_namespace = NIL;

-    /* add jrte to column namespace only */
-    addRTEtoQuery(pstate, jrte, false, false, true);
+    /* add jnsitem to column namespace only */
+    addNSItemtoQuery(pstate, jnsitem, false, false, true);

     /*
      * For now, we don't support resjunk sort clauses on the output of a
@@ -1757,7 +1759,7 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
                                           EXPR_KIND_ORDER_BY,
                                           false /* allow SQL92 rules */ );

-    /* restore namespace, remove jrte from rtable */
+    /* restore namespace, remove join RTE from rtable */
     pstate->p_namespace = sv_namespace;
     pstate->p_rtable = list_truncate(pstate->p_rtable, sv_rtable_length);

@@ -1869,7 +1871,7 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
         /* Process leaf SELECT */
         Query       *selectQuery;
         char        selectName[32];
-        RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY;
+        ParseNamespaceItem *nsitem;
         RangeTblRef *rtr;
         ListCell   *tl;

@@ -1926,19 +1928,17 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt,
          */
         snprintf(selectName, sizeof(selectName), "*SELECT* %d",
                  list_length(pstate->p_rtable) + 1);
-        rte = addRangeTableEntryForSubquery(pstate,
-                                            selectQuery,
-                                            makeAlias(selectName, NIL),
-                                            false,
-                                            false);
+        nsitem = addRangeTableEntryForSubquery(pstate,
+                                               selectQuery,
+                                               makeAlias(selectName, NIL),
+                                               false,
+                                               false);

         /*
          * Return a RangeTblRef to replace the SelectStmt in the set-op tree.
          */
         rtr = makeNode(RangeTblRef);
-        /* assume new rte is at end */
-        rtr->rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtr->rtindex, pstate->p_rtable));
+        rtr->rtindex = nsitem->p_rtindex;
         return (Node *) rtr;
     }
     else
@@ -2236,9 +2236,8 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
                                          stmt->relation->inh,
                                          true,
                                          ACL_UPDATE);
-
-    /* grab the namespace item made by setTargetTable */
-    nsitem = (ParseNamespaceItem *) llast(pstate->p_namespace);
+    nsitem = pstate->p_target_nsitem;
+    Assert(nsitem->p_rtindex == qry->resultRelation);

     /* subqueries in FROM cannot access the result relation */
     nsitem->p_lateral_only = true;
@@ -2297,7 +2296,7 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
         pstate->p_next_resno = RelationGetNumberOfAttributes(pstate->p_target_relation) + 1;

     /* Prepare non-junk columns for assignment to target table */
-    target_rte = pstate->p_target_rangetblentry;
+    target_rte = pstate->p_target_nsitem->p_rte;
     orig_tl = list_head(origTlist);

     foreach(tl, tlist)
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index ebbba2d..80d27f5 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -51,37 +51,33 @@
 #include "utils/rel.h"
 #include "utils/syscache.h"

-/* Convenience macro for the most common makeNamespaceItem() case */
-#define makeDefaultNSItem(rte, rti) \
-    makeNamespaceItem(rte, rti, true, true, false, true)

 static void extractRemainingColumns(List *common_colnames,
                                     List *src_colnames, List *src_colvars,
-                                    List **res_colnames, List **res_colvars);
+                                    ParseNamespaceColumn *src_nscolumns,
+                                    List **res_colnames, List **res_colvars,
+                                    ParseNamespaceColumn *res_nscolumns);
 static Node *transformJoinUsingClause(ParseState *pstate,
                                       RangeTblEntry *leftRTE, RangeTblEntry *rightRTE,
                                       List *leftVars, List *rightVars);
 static Node *transformJoinOnClause(ParseState *pstate, JoinExpr *j,
                                    List *namespace);
-static RangeTblEntry *getRTEForSpecialRelationTypes(ParseState *pstate,
-                                                    RangeVar *rv);
-static RangeTblEntry *transformTableEntry(ParseState *pstate, RangeVar *r);
-static RangeTblEntry *transformRangeSubselect(ParseState *pstate,
-                                              RangeSubselect *r);
-static RangeTblEntry *transformRangeFunction(ParseState *pstate,
-                                             RangeFunction *r);
-static RangeTblEntry *transformRangeTableFunc(ParseState *pstate,
-                                              RangeTableFunc *t);
+static ParseNamespaceItem *transformTableEntry(ParseState *pstate, RangeVar *r);
+static ParseNamespaceItem *transformRangeSubselect(ParseState *pstate,
+                                                   RangeSubselect *r);
+static ParseNamespaceItem *transformRangeFunction(ParseState *pstate,
+                                                  RangeFunction *r);
+static ParseNamespaceItem *transformRangeTableFunc(ParseState *pstate,
+                                                   RangeTableFunc *t);
 static TableSampleClause *transformRangeTableSample(ParseState *pstate,
                                                     RangeTableSample *rts);
+static ParseNamespaceItem *getNSItemForSpecialRelationTypes(ParseState *pstate,
+                                                            RangeVar *rv);
 static Node *transformFromClauseItem(ParseState *pstate, Node *n,
-                                     RangeTblEntry **top_rte, int *top_rti,
+                                     ParseNamespaceItem **top_nsitem,
                                      List **namespace);
 static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype,
                                 Var *l_colvar, Var *r_colvar);
-static ParseNamespaceItem *makeNamespaceItem(RangeTblEntry *rte, int rtindex,
-                                             bool rel_visible, bool cols_visible,
-                                             bool lateral_only, bool lateral_ok);
 static void setNamespaceColumnVisibility(List *namespace, bool cols_visible);
 static void setNamespaceLateralState(List *namespace,
                                      bool lateral_only, bool lateral_ok);
@@ -130,13 +126,11 @@ transformFromClause(ParseState *pstate, List *frmList)
     foreach(fl, frmList)
     {
         Node       *n = lfirst(fl);
-        RangeTblEntry *rte;
-        int            rtindex;
+        ParseNamespaceItem *nsitem;
         List       *namespace;

         n = transformFromClauseItem(pstate, n,
-                                    &rte,
-                                    &rtindex,
+                                    &nsitem,
                                     &namespace);

         checkNameSpaceConflicts(pstate, pstate->p_namespace, namespace);
@@ -183,8 +177,7 @@ int
 setTargetTable(ParseState *pstate, RangeVar *relation,
                bool inh, bool alsoSource, AclMode requiredPerms)
 {
-    RangeTblEntry *rte;
-    int            rtindex;
+    ParseNamespaceItem *nsitem;

     /*
      * ENRs hide tables of the same name, so we need to check for them first.
@@ -212,19 +205,14 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
                                                 RowExclusiveLock);

     /*
-     * Now build an RTE.
+     * Now build an RTE and a ParseNamespaceItem.
      */
-    rte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation,
-                                        RowExclusiveLock,
-                                        relation->alias, inh, false);
+    nsitem = addRangeTableEntryForRelation(pstate, pstate->p_target_relation,
+                                           RowExclusiveLock,
+                                           relation->alias, inh, false);

-    /* assume new rte is at end */
-    rtindex = list_length(pstate->p_rtable);
-    Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
-
-    /* remember the RTE as being the query target */
-    pstate->p_target_rangetblentry = rte;
-    pstate->p_target_rtindex = rtindex;
+    /* remember the RTE/nsitem as being the query target */
+    pstate->p_target_nsitem = nsitem;

     /*
      * Override addRangeTableEntry's default ACL_SELECT permissions check, and
@@ -235,28 +223,30 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
      * analysis, we will add the ACL_SELECT bit back again; see
      * markVarForSelectPriv and its callers.
      */
-    rte->requiredPerms = requiredPerms;
+    nsitem->p_rte->requiredPerms = requiredPerms;

     /*
      * If UPDATE/DELETE, add table to joinlist and namespace.
-     *
-     * Note: some callers know that they can find the new ParseNamespaceItem
-     * at the end of the pstate->p_namespace list.  This is a bit ugly but not
-     * worth complicating this function's signature for.
      */
     if (alsoSource)
-        addRTEtoQuery(pstate, rte, true, true, true);
+        addNSItemtoQuery(pstate, nsitem, true, true, true);

-    return rtindex;
+    return nsitem->p_rtindex;
 }

 /*
  * Extract all not-in-common columns from column lists of a source table
+ *
+ * We hand back new lists in *res_colnames and *res_colvars.  But
+ * res_nscolumns points to a caller-allocated array that we fill in
+ * the next few entries in.
  */
 static void
 extractRemainingColumns(List *common_colnames,
                         List *src_colnames, List *src_colvars,
-                        List **res_colnames, List **res_colvars)
+                        ParseNamespaceColumn *src_nscolumns,
+                        List **res_colnames, List **res_colvars,
+                        ParseNamespaceColumn *res_nscolumns)
 {
     List       *new_colnames = NIL;
     List       *new_colvars = NIL;
@@ -271,6 +261,14 @@ extractRemainingColumns(List *common_colnames,
         bool        match = false;
         ListCell   *cnames;

+        /*
+         * Ignore any dropped columns in the src_nscolumns input.  (The list
+         * inputs won't contain entries for dropped columns.)
+         */
+        while (src_nscolumns->p_varno == 0)
+            src_nscolumns++;
+
+        /* is current src column already accounted for in common_colnames? */
         foreach(cnames, common_colnames)
         {
             char       *ccolname = strVal(lfirst(cnames));
@@ -284,9 +282,15 @@ extractRemainingColumns(List *common_colnames,

         if (!match)
         {
+            /* Nope, so emit it as next output column */
             new_colnames = lappend(new_colnames, lfirst(lnames));
             new_colvars = lappend(new_colvars, lfirst(lvars));
+            /* Copy the input relation's nscolumn data for this column */
+            *res_nscolumns = *src_nscolumns;
+            res_nscolumns++;
         }
+
+        src_nscolumns++;
     }

     *res_colnames = new_colnames;
@@ -388,25 +392,20 @@ transformJoinOnClause(ParseState *pstate, JoinExpr *j, List *namespace)
 /*
  * transformTableEntry --- transform a RangeVar (simple relation reference)
  */
-static RangeTblEntry *
+static ParseNamespaceItem *
 transformTableEntry(ParseState *pstate, RangeVar *r)
 {
-    RangeTblEntry *rte;
-
-    /* We need only build a range table entry */
-    rte = addRangeTableEntry(pstate, r, r->alias, r->inh, true);
-
-    return rte;
+    /* addRangeTableEntry does all the work */
+    return addRangeTableEntry(pstate, r, r->alias, r->inh, true);
 }

 /*
  * transformRangeSubselect --- transform a sub-SELECT appearing in FROM
  */
-static RangeTblEntry *
+static ParseNamespaceItem *
 transformRangeSubselect(ParseState *pstate, RangeSubselect *r)
 {
     Query       *query;
-    RangeTblEntry *rte;

     /*
      * We require user to supply an alias for a subselect, per SQL92. To relax
@@ -454,29 +453,26 @@ transformRangeSubselect(ParseState *pstate, RangeSubselect *r)
         elog(ERROR, "unexpected non-SELECT command in subquery in FROM");

     /*
-     * OK, build an RTE for the subquery.
+     * OK, build an RTE and nsitem for the subquery.
      */
-    rte = addRangeTableEntryForSubquery(pstate,
-                                        query,
-                                        r->alias,
-                                        r->lateral,
-                                        true);
-
-    return rte;
+    return addRangeTableEntryForSubquery(pstate,
+                                         query,
+                                         r->alias,
+                                         r->lateral,
+                                         true);
 }


 /*
  * transformRangeFunction --- transform a function call appearing in FROM
  */
-static RangeTblEntry *
+static ParseNamespaceItem *
 transformRangeFunction(ParseState *pstate, RangeFunction *r)
 {
     List       *funcexprs = NIL;
     List       *funcnames = NIL;
     List       *coldeflists = NIL;
     bool        is_lateral;
-    RangeTblEntry *rte;
     ListCell   *lc;

     /*
@@ -677,13 +673,11 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r)
     is_lateral = r->lateral || contain_vars_of_level((Node *) funcexprs, 0);

     /*
-     * OK, build an RTE for the function.
+     * OK, build an RTE and nsitem for the function.
      */
-    rte = addRangeTableEntryForFunction(pstate,
-                                        funcnames, funcexprs, coldeflists,
-                                        r, is_lateral, true);
-
-    return rte;
+    return addRangeTableEntryForFunction(pstate,
+                                         funcnames, funcexprs, coldeflists,
+                                         r, is_lateral, true);
 }

 /*
@@ -694,13 +688,12 @@ transformRangeFunction(ParseState *pstate, RangeFunction *r)
  * row-generating expression, the column-generating expressions, and the
  * default value expressions.
  */
-static RangeTblEntry *
+static ParseNamespaceItem *
 transformRangeTableFunc(ParseState *pstate, RangeTableFunc *rtf)
 {
     TableFunc  *tf = makeNode(TableFunc);
     const char *constructName;
     Oid            docType;
-    RangeTblEntry *rte;
     bool        is_lateral;
     ListCell   *col;
     char      **names;
@@ -903,10 +896,8 @@ transformRangeTableFunc(ParseState *pstate, RangeTableFunc *rtf)
      */
     is_lateral = rtf->lateral || contain_vars_of_level((Node *) tf, 0);

-    rte = addRangeTableEntryForTableFunc(pstate,
-                                         tf, rtf->alias, is_lateral, true);
-
-    return rte;
+    return addRangeTableEntryForTableFunc(pstate,
+                                          tf, rtf->alias, is_lateral, true);
 }

 /*
@@ -1013,17 +1004,17 @@ transformRangeTableSample(ParseState *pstate, RangeTableSample *rts)
 }

 /*
- * getRTEForSpecialRelationTypes
+ * getNSItemForSpecialRelationTypes
  *
  * If given RangeVar refers to a CTE or an EphemeralNamedRelation,
- * build and return an appropriate RTE, otherwise return NULL
+ * build and return an appropriate ParseNamespaceItem, otherwise return NULL
  */
-static RangeTblEntry *
-getRTEForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)
+static ParseNamespaceItem *
+getNSItemForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)
 {
+    ParseNamespaceItem *nsitem;
     CommonTableExpr *cte;
     Index        levelsup;
-    RangeTblEntry *rte;

     /*
      * if it is a qualified name, it can't be a CTE or tuplestore reference
@@ -1033,13 +1024,13 @@ getRTEForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)

     cte = scanNameSpaceForCTE(pstate, rv->relname, &levelsup);
     if (cte)
-        rte = addRangeTableEntryForCTE(pstate, cte, levelsup, rv, true);
+        nsitem = addRangeTableEntryForCTE(pstate, cte, levelsup, rv, true);
     else if (scanNameSpaceForENR(pstate, rv->relname))
-        rte = addRangeTableEntryForENR(pstate, rv, true);
+        nsitem = addRangeTableEntryForENR(pstate, rv, true);
     else
-        rte = NULL;
+        nsitem = NULL;

-    return rte;
+    return nsitem;
 }

 /*
@@ -1053,11 +1044,9 @@ getRTEForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)
  * The function return value is the node to add to the jointree (a
  * RangeTblRef or JoinExpr).  Additional output parameters are:
  *
- * *top_rte: receives the RTE corresponding to the jointree item.
- * (We could extract this from the function return node, but it saves cycles
- * to pass it back separately.)
- *
- * *top_rti: receives the rangetable index of top_rte.  (Ditto.)
+ * *top_nsitem: receives the ParseNamespaceItem directly corresponding to the
+ * jointree item.  (This is only used during internal recursion, not by
+ * outside callers.)
  *
  * *namespace: receives a List of ParseNamespaceItems for the RTEs exposed
  * as table/column names by this item.  (The lateral_only flags in these items
@@ -1065,7 +1054,7 @@ getRTEForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)
  */
 static Node *
 transformFromClauseItem(ParseState *pstate, Node *n,
-                        RangeTblEntry **top_rte, int *top_rti,
+                        ParseNamespaceItem **top_nsitem,
                         List **namespace)
 {
     if (IsA(n, RangeVar))
@@ -1073,78 +1062,58 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         /* Plain relation reference, or perhaps a CTE reference */
         RangeVar   *rv = (RangeVar *) n;
         RangeTblRef *rtr;
-        RangeTblEntry *rte;
-        int            rtindex;
+        ParseNamespaceItem *nsitem;

         /* Check if it's a CTE or tuplestore reference */
-        rte = getRTEForSpecialRelationTypes(pstate, rv);
+        nsitem = getNSItemForSpecialRelationTypes(pstate, rv);

         /* if not found above, must be a table reference */
-        if (!rte)
-            rte = transformTableEntry(pstate, rv);
-
-        /* assume new rte is at end */
-        rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
-        *top_rte = rte;
-        *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
+        if (!nsitem)
+            nsitem = transformTableEntry(pstate, rv);
+
+        *top_nsitem = nsitem;
+        *namespace = list_make1(nsitem);
         rtr = makeNode(RangeTblRef);
-        rtr->rtindex = rtindex;
+        rtr->rtindex = nsitem->p_rtindex;
         return (Node *) rtr;
     }
     else if (IsA(n, RangeSubselect))
     {
         /* sub-SELECT is like a plain relation */
         RangeTblRef *rtr;
-        RangeTblEntry *rte;
-        int            rtindex;
-
-        rte = transformRangeSubselect(pstate, (RangeSubselect *) n);
-        /* assume new rte is at end */
-        rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
-        *top_rte = rte;
-        *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
+        ParseNamespaceItem *nsitem;
+
+        nsitem = transformRangeSubselect(pstate, (RangeSubselect *) n);
+        *top_nsitem = nsitem;
+        *namespace = list_make1(nsitem);
         rtr = makeNode(RangeTblRef);
-        rtr->rtindex = rtindex;
+        rtr->rtindex = nsitem->p_rtindex;
         return (Node *) rtr;
     }
     else if (IsA(n, RangeFunction))
     {
         /* function is like a plain relation */
         RangeTblRef *rtr;
-        RangeTblEntry *rte;
-        int            rtindex;
-
-        rte = transformRangeFunction(pstate, (RangeFunction *) n);
-        /* assume new rte is at end */
-        rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
-        *top_rte = rte;
-        *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
+        ParseNamespaceItem *nsitem;
+
+        nsitem = transformRangeFunction(pstate, (RangeFunction *) n);
+        *top_nsitem = nsitem;
+        *namespace = list_make1(nsitem);
         rtr = makeNode(RangeTblRef);
-        rtr->rtindex = rtindex;
+        rtr->rtindex = nsitem->p_rtindex;
         return (Node *) rtr;
     }
     else if (IsA(n, RangeTableFunc))
     {
         /* table function is like a plain relation */
         RangeTblRef *rtr;
-        RangeTblEntry *rte;
-        int            rtindex;
-
-        rte = transformRangeTableFunc(pstate, (RangeTableFunc *) n);
-        /* assume new rte is at end */
-        rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
-        *top_rte = rte;
-        *top_rti = rtindex;
-        *namespace = list_make1(makeDefaultNSItem(rte, rtindex));
+        ParseNamespaceItem *nsitem;
+
+        nsitem = transformRangeTableFunc(pstate, (RangeTableFunc *) n);
+        *top_nsitem = nsitem;
+        *namespace = list_make1(nsitem);
         rtr = makeNode(RangeTblRef);
-        rtr->rtindex = rtindex;
+        rtr->rtindex = nsitem->p_rtindex;
         return (Node *) rtr;
     }
     else if (IsA(n, RangeTableSample))
@@ -1152,19 +1121,17 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         /* TABLESAMPLE clause (wrapping some other valid FROM node) */
         RangeTableSample *rts = (RangeTableSample *) n;
         Node       *rel;
-        RangeTblRef *rtr;
         RangeTblEntry *rte;

         /* Recursively transform the contained relation */
         rel = transformFromClauseItem(pstate, rts->relation,
-                                      top_rte, top_rti, namespace);
-        /* Currently, grammar could only return a RangeVar as contained rel */
-        rtr = castNode(RangeTblRef, rel);
-        rte = rt_fetch(rtr->rtindex, pstate->p_rtable);
+                                      top_nsitem, namespace);
+        rte = (*top_nsitem)->p_rte;
         /* We only support this on plain relations and matviews */
-        if (rte->relkind != RELKIND_RELATION &&
-            rte->relkind != RELKIND_MATVIEW &&
-            rte->relkind != RELKIND_PARTITIONED_TABLE)
+        if (rte->rtekind != RTE_RELATION ||
+            (rte->relkind != RELKIND_RELATION &&
+             rte->relkind != RELKIND_MATVIEW &&
+             rte->relkind != RELKIND_PARTITIONED_TABLE))
             ereport(ERROR,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                      errmsg("TABLESAMPLE clause can only be applied to tables and materialized views"),
@@ -1172,16 +1139,15 @@ transformFromClauseItem(ParseState *pstate, Node *n,

         /* Transform TABLESAMPLE details and attach to the RTE */
         rte->tablesample = transformRangeTableSample(pstate, rts);
-        return (Node *) rtr;
+        return rel;
     }
     else if (IsA(n, JoinExpr))
     {
         /* A newfangled join expression */
         JoinExpr   *j = (JoinExpr *) n;
-        RangeTblEntry *l_rte;
-        RangeTblEntry *r_rte;
-        int            l_rtindex;
-        int            r_rtindex;
+        ParseNamespaceItem *nsitem;
+        ParseNamespaceItem *l_nsitem;
+        ParseNamespaceItem *r_nsitem;
         List       *l_namespace,
                    *r_namespace,
                    *my_namespace,
@@ -1191,9 +1157,10 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                    *l_colvars,
                    *r_colvars,
                    *res_colvars;
+        ParseNamespaceColumn *res_nscolumns;
+        int            res_colindex;
         bool        lateral_ok;
         int            sv_namespace_length;
-        RangeTblEntry *rte;
         int            k;

         /*
@@ -1201,8 +1168,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
          * it in this order for correct visibility of LATERAL references.
          */
         j->larg = transformFromClauseItem(pstate, j->larg,
-                                          &l_rte,
-                                          &l_rtindex,
+                                          &l_nsitem,
                                           &l_namespace);

         /*
@@ -1225,8 +1191,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,

         /* And now we can process the RHS */
         j->rarg = transformFromClauseItem(pstate, j->rarg,
-                                          &r_rte,
-                                          &r_rtindex,
+                                          &r_nsitem,
                                           &r_namespace);

         /* Remove the left-side RTEs from the namespace list again */
@@ -1248,12 +1213,10 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         /*
          * Extract column name and var lists from both subtrees
          *
-         * Note: expandRTE returns new lists, safe for me to modify
+         * Note: expandNSItemVars returns new lists, safe for me to modify
          */
-        expandRTE(l_rte, l_rtindex, 0, -1, false,
-                  &l_colnames, &l_colvars);
-        expandRTE(r_rte, r_rtindex, 0, -1, false,
-                  &r_colnames, &r_colvars);
+        l_colvars = expandNSItemVars(l_nsitem, 0, -1, &l_colnames);
+        r_colvars = expandNSItemVars(r_nsitem, 0, -1, &r_colnames);

         /*
          * Natural join does not explicitly specify columns; must generate
@@ -1302,6 +1265,12 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         res_colnames = NIL;
         res_colvars = NIL;

+        /* this may be larger than needed */
+        res_nscolumns = (ParseNamespaceColumn *)
+            palloc0((list_length(l_colnames) + list_length(r_colnames)) *
+                    sizeof(ParseNamespaceColumn));
+        res_colindex = 0;
+
         if (j->usingClause)
         {
             /*
@@ -1325,6 +1294,8 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                 int            r_index = -1;
                 Var           *l_colvar,
                            *r_colvar;
+                Node       *u_colvar;
+                ParseNamespaceColumn *res_nscolumn;

                 /* Check for USING(foo,foo) */
                 foreach(col, res_colnames)
@@ -1390,16 +1361,58 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                 r_usingvars = lappend(r_usingvars, r_colvar);

                 res_colnames = lappend(res_colnames, lfirst(ucol));
-                res_colvars = lappend(res_colvars,
-                                      buildMergedJoinVar(pstate,
-                                                         j->jointype,
-                                                         l_colvar,
-                                                         r_colvar));
+                u_colvar = buildMergedJoinVar(pstate,
+                                              j->jointype,
+                                              l_colvar,
+                                              r_colvar);
+                res_colvars = lappend(res_colvars, u_colvar);
+                res_nscolumn = res_nscolumns + res_colindex;
+                res_colindex++;
+                if (u_colvar == (Node *) l_colvar)
+                {
+                    /* Merged column is equivalent to left input */
+                    res_nscolumn->p_varno = l_colvar->varno;
+                    res_nscolumn->p_varattno = l_colvar->varattno;
+                    res_nscolumn->p_vartype = l_colvar->vartype;
+                    res_nscolumn->p_vartypmod = l_colvar->vartypmod;
+                    res_nscolumn->p_varcollid = l_colvar->varcollid;
+                    /* XXX these are not quite right, but doesn't matter yet */
+                    res_nscolumn->p_varnosyn = l_colvar->varno;
+                    res_nscolumn->p_varattnosyn = l_colvar->varattno;
+                }
+                else if (u_colvar == (Node *) r_colvar)
+                {
+                    /* Merged column is equivalent to right input */
+                    res_nscolumn->p_varno = r_colvar->varno;
+                    res_nscolumn->p_varattno = r_colvar->varattno;
+                    res_nscolumn->p_vartype = r_colvar->vartype;
+                    res_nscolumn->p_vartypmod = r_colvar->vartypmod;
+                    res_nscolumn->p_varcollid = r_colvar->varcollid;
+                    /* XXX these are not quite right, but doesn't matter yet */
+                    res_nscolumn->p_varnosyn = r_colvar->varno;
+                    res_nscolumn->p_varattnosyn = r_colvar->varattno;
+                }
+                else
+                {
+                    /*
+                     * Merged column is not semantically equivalent to either
+                     * input, so it needs to be referenced as the join output
+                     * column.  We don't know the join's varno yet, so we'll
+                     * replace these zeroes below.
+                     */
+                    res_nscolumn->p_varno = 0;
+                    res_nscolumn->p_varattno = res_colindex;
+                    res_nscolumn->p_vartype = exprType(u_colvar);
+                    res_nscolumn->p_vartypmod = exprTypmod(u_colvar);
+                    res_nscolumn->p_varcollid = exprCollation(u_colvar);
+                    res_nscolumn->p_varnosyn = 0;
+                    res_nscolumn->p_varattnosyn = res_colindex;
+                }
             }

             j->quals = transformJoinUsingClause(pstate,
-                                                l_rte,
-                                                r_rte,
+                                                l_nsitem->p_rte,
+                                                r_nsitem->p_rte,
                                                 l_usingvars,
                                                 r_usingvars);
         }
@@ -1416,10 +1429,16 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         /* Add remaining columns from each side to the output columns */
         extractRemainingColumns(res_colnames,
                                 l_colnames, l_colvars,
-                                &l_colnames, &l_colvars);
+                                l_nsitem->p_nscolumns,
+                                &l_colnames, &l_colvars,
+                                res_nscolumns + res_colindex);
+        res_colindex += list_length(l_colvars);
         extractRemainingColumns(res_colnames,
                                 r_colnames, r_colvars,
-                                &r_colnames, &r_colvars);
+                                r_nsitem->p_nscolumns,
+                                &r_colnames, &r_colvars,
+                                res_nscolumns + res_colindex);
+        res_colindex += list_length(r_colvars);
         res_colnames = list_concat(res_colnames, l_colnames);
         res_colvars = list_concat(res_colvars, l_colvars);
         res_colnames = list_concat(res_colnames, r_colnames);
@@ -1441,21 +1460,41 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         }

         /*
-         * Now build an RTE for the result of the join
+         * Now build an RTE and nsitem for the result of the join.
+         * res_nscolumns isn't totally done yet, but that's OK because
+         * addRangeTableEntryForJoin doesn't examine it, only store a pointer.
          */
-        rte = addRangeTableEntryForJoin(pstate,
-                                        res_colnames,
-                                        j->jointype,
-                                        res_colvars,
-                                        j->alias,
-                                        true);
+        nsitem = addRangeTableEntryForJoin(pstate,
+                                           res_colnames,
+                                           res_nscolumns,
+                                           j->jointype,
+                                           res_colvars,
+                                           j->alias,
+                                           true);

-        /* assume new rte is at end */
-        j->rtindex = list_length(pstate->p_rtable);
-        Assert(rte == rt_fetch(j->rtindex, pstate->p_rtable));
+        j->rtindex = nsitem->p_rtindex;

-        *top_rte = rte;
-        *top_rti = j->rtindex;
+        /*
+         * Now that we know the join RTE's rangetable index, we can fix up the
+         * res_nscolumns data in places where it should contain that.
+         */
+        Assert(res_colindex == list_length(nsitem->p_rte->eref->colnames));
+        for (k = 0; k < res_colindex; k++)
+        {
+            ParseNamespaceColumn *nscol = res_nscolumns + k;
+
+            /* fill in join RTI for merged columns */
+            if (nscol->p_varno == 0)
+                nscol->p_varno = j->rtindex;
+            if (nscol->p_varnosyn == 0)
+                nscol->p_varnosyn = j->rtindex;
+            /* if join has an alias, it syntactically hides all inputs */
+            if (j->alias)
+            {
+                nscol->p_varnosyn = j->rtindex;
+                nscol->p_varattnosyn = k + 1;
+            }
+        }

         /* make a matching link to the JoinExpr for later use */
         for (k = list_length(pstate->p_joinexprs) + 1; k < j->rtindex; k++)
@@ -1483,13 +1522,13 @@ transformFromClauseItem(ParseState *pstate, Node *n,
          * The join RTE itself is always made visible for unqualified column
          * names.  It's visible as a relation name only if it has an alias.
          */
-        *namespace = lappend(my_namespace,
-                             makeNamespaceItem(rte,
-                                               j->rtindex,
-                                               (j->alias != NULL),
-                                               true,
-                                               false,
-                                               true));
+        nsitem->p_rel_visible = (j->alias != NULL);
+        nsitem->p_cols_visible = true;
+        nsitem->p_lateral_only = false;
+        nsitem->p_lateral_ok = true;
+
+        *top_nsitem = nsitem;
+        *namespace = lappend(my_namespace, nsitem);

         return (Node *) j;
     }
@@ -1618,27 +1657,6 @@ buildMergedJoinVar(ParseState *pstate, JoinType jointype,
 }

 /*
- * makeNamespaceItem -
- *      Convenience subroutine to construct a ParseNamespaceItem.
- */
-static ParseNamespaceItem *
-makeNamespaceItem(RangeTblEntry *rte, int rtindex,
-                  bool rel_visible, bool cols_visible,
-                  bool lateral_only, bool lateral_ok)
-{
-    ParseNamespaceItem *nsitem;
-
-    nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
-    nsitem->p_rte = rte;
-    nsitem->p_rtindex = rtindex;
-    nsitem->p_rel_visible = rel_visible;
-    nsitem->p_cols_visible = cols_visible;
-    nsitem->p_lateral_only = lateral_only;
-    nsitem->p_lateral_ok = lateral_ok;
-    return nsitem;
-}
-
-/*
  * setNamespaceColumnVisibility -
  *      Convenience subroutine to update cols_visible flags in a namespace list.
  */
@@ -3163,8 +3181,8 @@ transformOnConflictArbiter(ParseState *pstate,
          */
         save_namespace = pstate->p_namespace;
         pstate->p_namespace = NIL;
-        addRTEtoQuery(pstate, pstate->p_target_rangetblentry,
-                      false, false, true);
+        addNSItemtoQuery(pstate, pstate->p_target_nsitem,
+                         false, false, true);

         if (infer->indexElems)
             *arbiterExpr = resolve_unique_index_expr(pstate, infer,
@@ -3189,7 +3207,7 @@ transformOnConflictArbiter(ParseState *pstate,
         if (infer->conname)
         {
             Oid            relid = RelationGetRelid(pstate->p_target_relation);
-            RangeTblEntry *rte = pstate->p_target_rangetblentry;
+            RangeTblEntry *rte = pstate->p_target_nsitem->p_rte;
             Bitmapset  *conattnos;

             conattnos = get_relation_constraint_attnos(relid, infer->conname,
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index a6c51a9..484298d 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1010,11 +1010,10 @@ coerce_record_to_complex(ParseState *pstate, Node *node,
         int            rtindex = ((Var *) node)->varno;
         int            sublevels_up = ((Var *) node)->varlevelsup;
         int            vlocation = ((Var *) node)->location;
-        RangeTblEntry *rte;
+        ParseNamespaceItem *nsitem;

-        rte = GetRTEByRangeTablePosn(pstate, rtindex, sublevels_up);
-        expandRTE(rte, rtindex, sublevels_up, vlocation, false,
-                  NULL, &args);
+        nsitem = GetNSItemByRangeTablePosn(pstate, rtindex, sublevels_up);
+        args = expandNSItemVars(nsitem, sublevels_up, vlocation, NULL);
     }
     else
         ereport(ERROR,
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 25e92de..06511df 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2656,8 +2656,8 @@ static Node *
 transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr)
 {
     /* CURRENT OF can only appear at top level of UPDATE/DELETE */
-    Assert(pstate->p_target_rtindex > 0);
-    cexpr->cvarno = pstate->p_target_rtindex;
+    Assert(pstate->p_target_nsitem != NULL);
+    cexpr->cvarno = pstate->p_target_nsitem->p_rtindex;

     /*
      * Check to see if the cursor name matches a parameter of type REFCURSOR.
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 4888311..113bb51 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -472,7 +472,8 @@ check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem,
                 (errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
                  errmsg("invalid reference to FROM-clause entry for table \"%s\"",
                         refname),
-                 (rte == pstate->p_target_rangetblentry) ?
+                 (pstate->p_target_nsitem != NULL &&
+                  rte == pstate->p_target_nsitem->p_rte) ?
                  errhint("There is an entry for table \"%s\", but it cannot be referenced from this part of the
query.",
                          refname) :
                  errdetail("The combining JOIN type must be INNER or LEFT for a LATERAL reference."),
@@ -669,9 +670,6 @@ scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
     RangeTblEntry *rte = nsitem->p_rte;
     int            attnum;
     Var           *var;
-    Oid            vartypeid;
-    int32        vartypmod;
-    Oid            varcollid;

     /*
      * Scan the RTE's column names (or aliases) for a match.  Complain if
@@ -703,11 +701,39 @@ scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
                  parser_errposition(pstate, location)));

     /* Found a valid match, so build a Var */
-    get_rte_attribute_type(rte, attnum,
-                           &vartypeid, &vartypmod, &varcollid);
-    var = makeVar(nsitem->p_rtindex, attnum,
-                  vartypeid, vartypmod, varcollid,
-                  sublevels_up);
+    if (attnum > InvalidAttrNumber)
+    {
+        /* Get attribute data from the ParseNamespaceColumn array */
+        ParseNamespaceColumn *nscol = &nsitem->p_nscolumns[attnum - 1];
+
+        /* Complain if dropped column.  See notes in scanRTEForColumn. */
+        if (nscol->p_varno == 0)
+            ereport(ERROR,
+                    (errcode(ERRCODE_UNDEFINED_COLUMN),
+                     errmsg("column \"%s\" of relation \"%s\" does not exist",
+                            colname,
+                            rte->eref->aliasname)));
+
+        var = makeVar(nsitem->p_rtindex,
+                      attnum,
+                      nscol->p_vartype,
+                      nscol->p_vartypmod,
+                      nscol->p_varcollid,
+                      sublevels_up);
+    }
+    else
+    {
+        /* System column, so use predetermined type data */
+        const FormData_pg_attribute *sysatt;
+
+        sysatt = SystemAttributeDefinition(attnum);
+        var = makeVar(nsitem->p_rtindex,
+                      attnum,
+                      sysatt->atttypid,
+                      sysatt->atttypmod,
+                      sysatt->attcollation,
+                      sublevels_up);
+    }
     var->location = location;

     /* Require read access to the column */
@@ -753,11 +779,9 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
      * don't bother to test for that case here.
      *
      * Should this somehow go wrong and we try to access a dropped column,
-     * we'll still catch it by virtue of the checks in
-     * get_rte_attribute_type(), which is called by scanNSItemForColumn().
-     * That routine has to do a cache lookup anyway, so the check there is
-     * cheap.  Callers interested in finding match with shortest distance need
-     * to defend against this directly, though.
+     * we'll still catch it by virtue of the check in scanNSItemForColumn().
+     * Callers interested in finding match with shortest distance need to
+     * defend against this directly, though.
      */
     foreach(c, rte->eref->colnames)
     {
@@ -1201,6 +1225,121 @@ chooseScalarFunctionAlias(Node *funcexpr, char *funcname,
 }

 /*
+ * buildNSItemFromTupleDesc
+ *        Build a ParseNamespaceItem, given a tupdesc describing the columns.
+ *
+ * rte: the new RangeTblEntry for the rel
+ * rtindex: its index in the rangetable list
+ * tupdesc: the physical column information
+ */
+static ParseNamespaceItem *
+buildNSItemFromTupleDesc(RangeTblEntry *rte, Index rtindex, TupleDesc tupdesc)
+{
+    ParseNamespaceItem *nsitem;
+    ParseNamespaceColumn *nscolumns;
+    int            maxattrs = tupdesc->natts;
+    int            varattno;
+
+    /* colnames must have the same number of entries as the nsitem */
+    Assert(maxattrs == list_length(rte->eref->colnames));
+
+    /* extract per-column data from the tupdesc */
+    nscolumns = (ParseNamespaceColumn *)
+        palloc0(maxattrs * sizeof(ParseNamespaceColumn));
+
+    for (varattno = 0; varattno < maxattrs; varattno++)
+    {
+        Form_pg_attribute attr = TupleDescAttr(tupdesc, varattno);
+
+        /* For a dropped column, just leave the entry as zeroes */
+        if (attr->attisdropped)
+            continue;
+
+        nscolumns[varattno].p_varno = rtindex;
+        nscolumns[varattno].p_varattno = varattno + 1;
+        nscolumns[varattno].p_vartype = attr->atttypid;
+        nscolumns[varattno].p_vartypmod = attr->atttypmod;
+        nscolumns[varattno].p_varcollid = attr->attcollation;
+        nscolumns[varattno].p_varnosyn = rtindex;
+        nscolumns[varattno].p_varattnosyn = varattno + 1;
+    }
+
+    /* ... and build the nsitem */
+    nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
+    nsitem->p_rte = rte;
+    nsitem->p_rtindex = rtindex;
+    nsitem->p_nscolumns = nscolumns;
+    /* set default visibility flags; might get changed later */
+    nsitem->p_rel_visible = true;
+    nsitem->p_cols_visible = true;
+    nsitem->p_lateral_only = false;
+    nsitem->p_lateral_ok = true;
+
+    return nsitem;
+}
+
+/*
+ * buildNSItemFromLists
+ *        Build a ParseNamespaceItem, given column type information in lists.
+ *
+ * rte: the new RangeTblEntry for the rel
+ * rtindex: its index in the rangetable list
+ * coltypes: per-column datatype OIDs
+ * coltypmods: per-column type modifiers
+ * colcollation: per-column collation OIDs
+ */
+static ParseNamespaceItem *
+buildNSItemFromLists(RangeTblEntry *rte, Index rtindex,
+                     List *coltypes, List *coltypmods, List *colcollations)
+{
+    ParseNamespaceItem *nsitem;
+    ParseNamespaceColumn *nscolumns;
+    int            maxattrs = list_length(coltypes);
+    int            varattno;
+    ListCell   *lct;
+    ListCell   *lcm;
+    ListCell   *lcc;
+
+    /* colnames must have the same number of entries as the nsitem */
+    Assert(maxattrs == list_length(rte->eref->colnames));
+
+    Assert(maxattrs == list_length(coltypmods));
+    Assert(maxattrs == list_length(colcollations));
+
+    /* extract per-column data from the lists */
+    nscolumns = (ParseNamespaceColumn *)
+        palloc0(maxattrs * sizeof(ParseNamespaceColumn));
+
+    varattno = 0;
+    forthree(lct, coltypes,
+             lcm, coltypmods,
+             lcc, colcollations)
+    {
+        nscolumns[varattno].p_varno = rtindex;
+        nscolumns[varattno].p_varattno = varattno + 1;
+        nscolumns[varattno].p_vartype = lfirst_oid(lct);
+        nscolumns[varattno].p_vartypmod = lfirst_int(lcm);
+        nscolumns[varattno].p_varcollid = lfirst_oid(lcc);
+        nscolumns[varattno].p_varnosyn = rtindex;
+        nscolumns[varattno].p_varattnosyn = varattno + 1;
+        varattno++;
+    }
+
+    /* ... and build the nsitem */
+    nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
+    nsitem->p_rte = rte;
+    nsitem->p_rtindex = rtindex;
+    nsitem->p_nscolumns = nscolumns;
+    /* set default visibility flags; might get changed later */
+    nsitem->p_rel_visible = true;
+    nsitem->p_cols_visible = true;
+    nsitem->p_lateral_only = false;
+    nsitem->p_lateral_ok = true;
+
+    return nsitem;
+}
+
+/*
  * Open a table during parse analysis
  *
  * This is essentially just the same as table_openrv(), except that it caters
@@ -1255,11 +1394,15 @@ parserOpenTable(ParseState *pstate, const RangeVar *relation, int lockmode)

 /*
  * Add an entry for a relation to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
+ *
+ * We do not link the ParseNamespaceItem into the pstate here; it's the
+ * caller's job to do that in the appropriate way.
  *
  * Note: formerly this checked for refname conflicts, but that's wrong.
  * Caller is responsible for checking for conflicts in the appropriate scope.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntry(ParseState *pstate,
                    RangeVar *relation,
                    Alias *alias,
@@ -1270,6 +1413,7 @@ addRangeTableEntry(ParseState *pstate,
     char       *refname = alias ? alias->aliasname : relation->relname;
     LOCKMODE    lockmode;
     Relation    rel;
+    ParseNamespaceItem *nsitem;

     Assert(pstate != NULL);

@@ -1302,13 +1446,6 @@ addRangeTableEntry(ParseState *pstate,
     buildRelationAliases(rel->rd_att, alias, rte->eref);

     /*
-     * Drop the rel refcount, but keep the access lock till end of transaction
-     * so that the table can't be deleted or have its schema modified
-     * underneath us.
-     */
-    table_close(rel, NoLock);
-
-    /*
      * Set flags and access permissions.
      *
      * The initial default on access checks is always check-for-READ-access,
@@ -1326,16 +1463,32 @@ addRangeTableEntry(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    nsitem = buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable),
+                                      rel->rd_att);
+
+    /*
+     * Drop the rel refcount, but keep the access lock till end of transaction
+     * so that the table can't be deleted or have its schema modified
+     * underneath us.
+     */
+    table_close(rel, NoLock);
+
+    return nsitem;
 }

 /*
  * Add an entry for a relation to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is just like addRangeTableEntry() except that it makes an RTE
  * given an already-open relation instead of a RangeVar reference.
@@ -1349,7 +1502,7 @@ addRangeTableEntry(ParseState *pstate,
  * would require importing storage/lock.h into parse_relation.h.  Since
  * LOCKMODE is typedef'd as int anyway, that seems like overkill.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForRelation(ParseState *pstate,
                               Relation rel,
                               int lockmode,
@@ -1398,21 +1551,28 @@ addRangeTableEntryForRelation(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable),
+                                    rel->rd_att);
 }

 /*
  * Add an entry for a subquery to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is just like addRangeTableEntry() except that it makes a subquery RTE.
  * Note that an alias clause *must* be supplied.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForSubquery(ParseState *pstate,
                               Query *subquery,
                               Alias *alias,
@@ -1423,6 +1583,9 @@ addRangeTableEntryForSubquery(ParseState *pstate,
     char       *refname = alias->aliasname;
     Alias       *eref;
     int            numaliases;
+    List       *coltypes,
+               *coltypmods,
+               *colcollations;
     int            varattno;
     ListCell   *tlistitem;

@@ -1435,7 +1598,8 @@ addRangeTableEntryForSubquery(ParseState *pstate,
     eref = copyObject(alias);
     numaliases = list_length(eref->colnames);

-    /* fill in any unspecified alias columns */
+    /* fill in any unspecified alias columns, and extract column type info */
+    coltypes = coltypmods = colcollations = NIL;
     varattno = 0;
     foreach(tlistitem, subquery->targetList)
     {
@@ -1452,6 +1616,12 @@ addRangeTableEntryForSubquery(ParseState *pstate,
             attrname = pstrdup(te->resname);
             eref->colnames = lappend(eref->colnames, makeString(attrname));
         }
+        coltypes = lappend_oid(coltypes,
+                               exprType((Node *) te->expr));
+        coltypmods = lappend_int(coltypmods,
+                                 exprTypmod((Node *) te->expr));
+        colcollations = lappend_oid(colcollations,
+                                    exprCollation((Node *) te->expr));
     }
     if (varattno < numaliases)
         ereport(ERROR,
@@ -1478,21 +1648,27 @@ addRangeTableEntryForSubquery(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromLists(rte, list_length(pstate->p_rtable),
+                                coltypes, coltypmods, colcollations);
 }

 /*
  * Add an entry for a function (or functions) to the pstate's range table
- * (p_rtable).
+ * (p_rtable).  Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is just like addRangeTableEntry() except that it makes a function RTE.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForFunction(ParseState *pstate,
                               List *funcnames,
                               List *funcexprs,
@@ -1742,20 +1918,27 @@ addRangeTableEntryForFunction(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable),
+                                    tupdesc);
 }

 /*
  * Add an entry for a table function to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is much like addRangeTableEntry() except that it makes a tablefunc RTE.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForTableFunc(ParseState *pstate,
                                TableFunc *tf,
                                Alias *alias,
@@ -1806,20 +1989,28 @@ addRangeTableEntryForTableFunc(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromLists(rte, list_length(pstate->p_rtable),
+                                rte->coltypes, rte->coltypmods,
+                                rte->colcollations);
 }

 /*
  * Add an entry for a VALUES list to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is much like addRangeTableEntry() except that it makes a values RTE.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForValues(ParseState *pstate,
                             List *exprs,
                             List *coltypes,
@@ -1885,22 +2076,33 @@ addRangeTableEntryForValues(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromLists(rte, list_length(pstate->p_rtable),
+                                rte->coltypes, rte->coltypmods,
+                                rte->colcollations);
 }

 /*
  * Add an entry for a join to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is much like addRangeTableEntry() except that it makes a join RTE.
+ * Also, it's more convenient for the caller to construct the
+ * ParseNamespaceColumn array, so we pass that in.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForJoin(ParseState *pstate,
                           List *colnames,
+                          ParseNamespaceColumn *nscolumns,
                           JoinType jointype,
                           List *aliasvars,
                           Alias *alias,
@@ -1909,6 +2111,7 @@ addRangeTableEntryForJoin(ParseState *pstate,
     RangeTblEntry *rte = makeNode(RangeTblEntry);
     Alias       *eref;
     int            numaliases;
+    ParseNamespaceItem *nsitem;

     Assert(pstate != NULL);

@@ -1956,20 +2159,36 @@ addRangeTableEntryForJoin(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
+    nsitem->p_rte = rte;
+    nsitem->p_rtindex = list_length(pstate->p_rtable);
+    nsitem->p_nscolumns = nscolumns;
+    /* set default visibility flags; might get changed later */
+    nsitem->p_rel_visible = true;
+    nsitem->p_cols_visible = true;
+    nsitem->p_lateral_only = false;
+    nsitem->p_lateral_ok = true;
+
+    return nsitem;
 }

 /*
  * Add an entry for a CTE reference to the pstate's range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * This is much like addRangeTableEntry() except that it makes a CTE RTE.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForCTE(ParseState *pstate,
                          CommonTableExpr *cte,
                          Index levelsup,
@@ -2059,17 +2278,25 @@ addRangeTableEntryForCTE(ParseState *pstate,
     rte->extraUpdatedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromLists(rte, list_length(pstate->p_rtable),
+                                rte->coltypes, rte->coltypmods,
+                                rte->colcollations);
 }

 /*
  * Add an entry for an ephemeral named relation reference to the pstate's
  * range table (p_rtable).
+ * Then, construct and return a ParseNamespaceItem for the new RTE.
  *
  * It is expected that the RangeVar, which up until now is only known to be an
  * ephemeral named relation, will (in conjunction with the QueryEnvironment in
@@ -2079,7 +2306,7 @@ addRangeTableEntryForCTE(ParseState *pstate,
  * This is much like addRangeTableEntry() except that it makes an RTE for an
  * ephemeral named relation.
  */
-RangeTblEntry *
+ParseNamespaceItem *
 addRangeTableEntryForENR(ParseState *pstate,
                          RangeVar *rv,
                          bool inFromCl)
@@ -2164,12 +2391,18 @@ addRangeTableEntryForENR(ParseState *pstate,
     rte->selectedCols = NULL;

     /*
-     * Add completed RTE to pstate's range table list, but not to join list
-     * nor namespace --- caller must do that if appropriate.
+     * Add completed RTE to pstate's range table list, so that we know its
+     * index.  But we don't add it to the join list --- caller must do that if
+     * appropriate.
      */
     pstate->p_rtable = lappend(pstate->p_rtable, rte);

-    return rte;
+    /*
+     * Build a ParseNamespaceItem, but don't add it to the pstate's namespace
+     * list --- caller must do that if appropriate.
+     */
+    return buildNSItemFromTupleDesc(rte, list_length(pstate->p_rtable),
+                                    tupdesc);
 }


@@ -2221,49 +2454,26 @@ isLockedRefname(ParseState *pstate, const char *refname)
 }

 /*
- * Add the given RTE as a top-level entry in the pstate's join list
+ * Add the given nsitem/RTE as a top-level entry in the pstate's join list
  * and/or namespace list.  (We assume caller has checked for any
- * namespace conflicts.)  The RTE is always marked as unconditionally
+ * namespace conflicts.)  The nsitem is always marked as unconditionally
  * visible, that is, not LATERAL-only.
- *
- * Note: some callers know that they can find the new ParseNamespaceItem
- * at the end of the pstate->p_namespace list.  This is a bit ugly but not
- * worth complicating this function's signature for.
  */
 void
-addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,
-              bool addToJoinList,
-              bool addToRelNameSpace, bool addToVarNameSpace)
+addNSItemtoQuery(ParseState *pstate, ParseNamespaceItem *nsitem,
+                 bool addToJoinList,
+                 bool addToRelNameSpace, bool addToVarNameSpace)
 {
-    int            rtindex;
-
-    /*
-     * Most callers have just added the RTE to the rangetable, so it's likely
-     * to be the last entry.  Hence, it's a good idea to search the rangetable
-     * back-to-front.
-     */
-    for (rtindex = list_length(pstate->p_rtable); rtindex > 0; rtindex--)
-    {
-        if (rte == rt_fetch(rtindex, pstate->p_rtable))
-            break;
-    }
-    if (rtindex <= 0)
-        elog(ERROR, "RTE not found (internal error)");
-
     if (addToJoinList)
     {
         RangeTblRef *rtr = makeNode(RangeTblRef);

-        rtr->rtindex = rtindex;
+        rtr->rtindex = nsitem->p_rtindex;
         pstate->p_joinlist = lappend(pstate->p_joinlist, rtr);
     }
     if (addToRelNameSpace || addToVarNameSpace)
     {
-        ParseNamespaceItem *nsitem;
-
-        nsitem = (ParseNamespaceItem *) palloc(sizeof(ParseNamespaceItem));
-        nsitem->p_rte = rte;
-        nsitem->p_rtindex = rtindex;
+        /* Set the new nsitem's visibility flags correctly */
         nsitem->p_rel_visible = addToRelNameSpace;
         nsitem->p_cols_visible = addToVarNameSpace;
         nsitem->p_lateral_only = false;
@@ -2721,6 +2931,61 @@ expandTupleDesc(TupleDesc tupdesc, Alias *eref, int count, int offset,
 }

 /*
+ * expandNSItemVars
+ *      Produce a list of Vars, and optionally a list of column names,
+ *      for the non-dropped columns of the nsitem.
+ *
+ * The emitted Vars are marked with the given sublevels_up and location.
+ *
+ * If colnames isn't NULL, a list of String items for the columns is stored
+ * there; note that it's just a subset of the RTE's eref list, and hence
+ * the list elements mustn't be modified.
+ */
+List *
+expandNSItemVars(ParseNamespaceItem *nsitem,
+                 int sublevels_up, int location,
+                 List **colnames)
+{
+    List       *result = NIL;
+    int            colindex;
+    ListCell   *lc;
+
+    if (colnames)
+        *colnames = NIL;
+    colindex = 0;
+    foreach(lc, nsitem->p_rte->eref->colnames)
+    {
+        Value       *colnameval = (Value *) lfirst(lc);
+        const char *colname = strVal(colnameval);
+        ParseNamespaceColumn *nscol = nsitem->p_nscolumns + colindex;
+
+        if (colname[0])
+        {
+            Var           *var;
+
+            Assert(nscol->p_varno > 0);
+            var = makeVar(nsitem->p_rtindex,
+                          colindex + 1,
+                          nscol->p_vartype,
+                          nscol->p_vartypmod,
+                          nscol->p_varcollid,
+                          sublevels_up);
+            var->location = location;
+            result = lappend(result, var);
+            if (colnames)
+                *colnames = lappend(*colnames, colnameval);
+        }
+        else
+        {
+            /* dropped column, ignore */
+            Assert(nscol->p_varno == 0);
+        }
+        colindex++;
+    }
+    return result;
+}
+
+/*
  * expandNSItemAttrs -
  *      Workhorse for "*" expansion: produce a list of targetentries
  *      for the attributes of the nsitem
@@ -2739,8 +3004,7 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
                *var;
     List       *te_list = NIL;

-    expandRTE(rte, nsitem->p_rtindex, sublevels_up, location, false,
-              &names, &vars);
+    vars = expandNSItemVars(nsitem, sublevels_up, location, &names);

     /*
      * Require read access to the table.  This is normally redundant with the
@@ -2816,204 +3080,6 @@ get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
 }

 /*
- * get_rte_attribute_type
- *        Get attribute type/typmod/collation information from a RangeTblEntry
- */
-void
-get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
-                       Oid *vartype, int32 *vartypmod, Oid *varcollid)
-{
-    switch (rte->rtekind)
-    {
-        case RTE_RELATION:
-            {
-                /* Plain relation RTE --- get the attribute's type info */
-                HeapTuple    tp;
-                Form_pg_attribute att_tup;
-
-                tp = SearchSysCache2(ATTNUM,
-                                     ObjectIdGetDatum(rte->relid),
-                                     Int16GetDatum(attnum));
-                if (!HeapTupleIsValid(tp))    /* shouldn't happen */
-                    elog(ERROR, "cache lookup failed for attribute %d of relation %u",
-                         attnum, rte->relid);
-                att_tup = (Form_pg_attribute) GETSTRUCT(tp);
-
-                /*
-                 * If dropped column, pretend it ain't there.  See notes in
-                 * scanRTEForColumn.
-                 */
-                if (att_tup->attisdropped)
-                    ereport(ERROR,
-                            (errcode(ERRCODE_UNDEFINED_COLUMN),
-                             errmsg("column \"%s\" of relation \"%s\" does not exist",
-                                    NameStr(att_tup->attname),
-                                    get_rel_name(rte->relid))));
-                *vartype = att_tup->atttypid;
-                *vartypmod = att_tup->atttypmod;
-                *varcollid = att_tup->attcollation;
-                ReleaseSysCache(tp);
-            }
-            break;
-        case RTE_SUBQUERY:
-            {
-                /* Subselect RTE --- get type info from subselect's tlist */
-                TargetEntry *te = get_tle_by_resno(rte->subquery->targetList,
-                                                   attnum);
-
-                if (te == NULL || te->resjunk)
-                    elog(ERROR, "subquery %s does not have attribute %d",
-                         rte->eref->aliasname, attnum);
-                *vartype = exprType((Node *) te->expr);
-                *vartypmod = exprTypmod((Node *) te->expr);
-                *varcollid = exprCollation((Node *) te->expr);
-            }
-            break;
-        case RTE_FUNCTION:
-            {
-                /* Function RTE */
-                ListCell   *lc;
-                int            atts_done = 0;
-
-                /* Identify which function covers the requested column */
-                foreach(lc, rte->functions)
-                {
-                    RangeTblFunction *rtfunc = (RangeTblFunction *) lfirst(lc);
-
-                    if (attnum > atts_done &&
-                        attnum <= atts_done + rtfunc->funccolcount)
-                    {
-                        TypeFuncClass functypclass;
-                        Oid            funcrettype;
-                        TupleDesc    tupdesc;
-
-                        attnum -= atts_done;    /* now relative to this func */
-                        functypclass = get_expr_result_type(rtfunc->funcexpr,
-                                                            &funcrettype,
-                                                            &tupdesc);
-
-                        if (functypclass == TYPEFUNC_COMPOSITE ||
-                            functypclass == TYPEFUNC_COMPOSITE_DOMAIN)
-                        {
-                            /* Composite data type, e.g. a table's row type */
-                            Form_pg_attribute att_tup;
-
-                            Assert(tupdesc);
-                            Assert(attnum <= tupdesc->natts);
-                            att_tup = TupleDescAttr(tupdesc, attnum - 1);
-
-                            /*
-                             * If dropped column, pretend it ain't there.  See
-                             * notes in scanRTEForColumn.
-                             */
-                            if (att_tup->attisdropped)
-                                ereport(ERROR,
-                                        (errcode(ERRCODE_UNDEFINED_COLUMN),
-                                         errmsg("column \"%s\" of relation \"%s\" does not exist",
-                                                NameStr(att_tup->attname),
-                                                rte->eref->aliasname)));
-                            *vartype = att_tup->atttypid;
-                            *vartypmod = att_tup->atttypmod;
-                            *varcollid = att_tup->attcollation;
-                        }
-                        else if (functypclass == TYPEFUNC_SCALAR)
-                        {
-                            /* Base data type, i.e. scalar */
-                            *vartype = funcrettype;
-                            *vartypmod = -1;
-                            *varcollid = exprCollation(rtfunc->funcexpr);
-                        }
-                        else if (functypclass == TYPEFUNC_RECORD)
-                        {
-                            *vartype = list_nth_oid(rtfunc->funccoltypes,
-                                                    attnum - 1);
-                            *vartypmod = list_nth_int(rtfunc->funccoltypmods,
-                                                      attnum - 1);
-                            *varcollid = list_nth_oid(rtfunc->funccolcollations,
-                                                      attnum - 1);
-                        }
-                        else
-                        {
-                            /*
-                             * addRangeTableEntryForFunction should've caught
-                             * this
-                             */
-                            elog(ERROR, "function in FROM has unsupported return type");
-                        }
-                        return;
-                    }
-                    atts_done += rtfunc->funccolcount;
-                }
-
-                /* If we get here, must be looking for the ordinality column */
-                if (rte->funcordinality && attnum == atts_done + 1)
-                {
-                    *vartype = INT8OID;
-                    *vartypmod = -1;
-                    *varcollid = InvalidOid;
-                    return;
-                }
-
-                /* this probably can't happen ... */
-                ereport(ERROR,
-                        (errcode(ERRCODE_UNDEFINED_COLUMN),
-                         errmsg("column %d of relation \"%s\" does not exist",
-                                attnum,
-                                rte->eref->aliasname)));
-            }
-            break;
-        case RTE_JOIN:
-            {
-                /*
-                 * Join RTE --- get type info from join RTE's alias variable
-                 */
-                Node       *aliasvar;
-
-                Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars));
-                aliasvar = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
-                Assert(aliasvar != NULL);
-                *vartype = exprType(aliasvar);
-                *vartypmod = exprTypmod(aliasvar);
-                *varcollid = exprCollation(aliasvar);
-            }
-            break;
-        case RTE_TABLEFUNC:
-        case RTE_VALUES:
-        case RTE_CTE:
-        case RTE_NAMEDTUPLESTORE:
-            {
-                /*
-                 * tablefunc, VALUES, CTE, or ENR RTE --- get type info from
-                 * lists in the RTE
-                 */
-                Assert(attnum > 0 && attnum <= list_length(rte->coltypes));
-                *vartype = list_nth_oid(rte->coltypes, attnum - 1);
-                *vartypmod = list_nth_int(rte->coltypmods, attnum - 1);
-                *varcollid = list_nth_oid(rte->colcollations, attnum - 1);
-
-                /* For ENR, better check for dropped column */
-                if (!OidIsValid(*vartype))
-                    ereport(ERROR,
-                            (errcode(ERRCODE_UNDEFINED_COLUMN),
-                             errmsg("column %d of relation \"%s\" does not exist",
-                                    attnum,
-                                    rte->eref->aliasname)));
-            }
-            break;
-        case RTE_RESULT:
-            /* this probably can't happen ... */
-            ereport(ERROR,
-                    (errcode(ERRCODE_UNDEFINED_COLUMN),
-                     errmsg("column %d of relation \"%s\" does not exist",
-                            attnum,
-                            rte->eref->aliasname)));
-            break;
-        default:
-            elog(ERROR, "unrecognized RTE kind: %d", (int) rte->rtekind);
-    }
-}
-
-/*
  * get_rte_attribute_is_dropped
  *        Check whether attempted attribute ref is to a dropped column
  */
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 46fe6c6..967808b 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -551,7 +551,7 @@ transformAssignedExpr(ParseState *pstate,
              */
             Var           *var;

-            var = makeVar(pstate->p_target_rtindex, attrno,
+            var = makeVar(pstate->p_target_nsitem->p_rtindex, attrno,
                           attrtype, attrtypmod, attrcollation, 0);
             var->location = location;

@@ -1359,8 +1359,7 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem,
         List       *vars;
         ListCell   *l;

-        expandRTE(rte, nsitem->p_rtindex, sublevels_up, location, false,
-                  NULL, &vars);
+        vars = expandNSItemVars(nsitem, sublevels_up, location, NULL);

         /*
          * Require read access to the table.  This is normally redundant with
@@ -1496,6 +1495,12 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
     Assert(IsA(var, Var));
     Assert(var->vartype == RECORDOID);

+    /*
+     * Note: it's tempting to use GetNSItemByRangeTablePosn here so that we
+     * can use expandNSItemVars instead of expandRTE; but that does not work
+     * for some of the recursion cases below, where we have consed up a
+     * ParseState that lacks p_namespace data.
+     */
     netlevelsup = var->varlevelsup + levelsup;
     rte = GetRTEByRangeTablePosn(pstate, var->varno, netlevelsup);
     attnum = var->varattno;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 45bb31e..b783cb8 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -2598,7 +2598,7 @@ IndexStmt *
 transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString)
 {
     ParseState *pstate;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;
     ListCell   *l;
     Relation    rel;

@@ -2622,12 +2622,12 @@ transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString)
      * relation, but we still need to open it.
      */
     rel = relation_open(relid, NoLock);
-    rte = addRangeTableEntryForRelation(pstate, rel,
-                                        AccessShareLock,
-                                        NULL, false, true);
+    nsitem = addRangeTableEntryForRelation(pstate, rel,
+                                           AccessShareLock,
+                                           NULL, false, true);

     /* no to join list, yes to namespaces */
-    addRTEtoQuery(pstate, rte, false, true, true);
+    addNSItemtoQuery(pstate, nsitem, false, true, true);

     /* take care of the where clause */
     if (stmt->whereClause)
@@ -2707,8 +2707,8 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
 {
     Relation    rel;
     ParseState *pstate;
-    RangeTblEntry *oldrte;
-    RangeTblEntry *newrte;
+    ParseNamespaceItem *oldnsitem;
+    ParseNamespaceItem *newnsitem;

     /*
      * To avoid deadlock, make sure the first thing we do is grab
@@ -2729,20 +2729,20 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,

     /*
      * NOTE: 'OLD' must always have a varno equal to 1 and 'NEW' equal to 2.
-     * Set up their RTEs in the main pstate for use in parsing the rule
-     * qualification.
+     * Set up their ParseNamespaceItems in the main pstate for use in parsing
+     * the rule qualification.
      */
-    oldrte = addRangeTableEntryForRelation(pstate, rel,
-                                           AccessShareLock,
-                                           makeAlias("old", NIL),
-                                           false, false);
-    newrte = addRangeTableEntryForRelation(pstate, rel,
-                                           AccessShareLock,
-                                           makeAlias("new", NIL),
-                                           false, false);
+    oldnsitem = addRangeTableEntryForRelation(pstate, rel,
+                                              AccessShareLock,
+                                              makeAlias("old", NIL),
+                                              false, false);
+    newnsitem = addRangeTableEntryForRelation(pstate, rel,
+                                              AccessShareLock,
+                                              makeAlias("new", NIL),
+                                              false, false);
     /* Must override addRangeTableEntry's default access-check flags */
-    oldrte->requiredPerms = 0;
-    newrte->requiredPerms = 0;
+    oldnsitem->p_rte->requiredPerms = 0;
+    newnsitem->p_rte->requiredPerms = 0;

     /*
      * They must be in the namespace too for lookup purposes, but only add the
@@ -2754,17 +2754,17 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
     switch (stmt->event)
     {
         case CMD_SELECT:
-            addRTEtoQuery(pstate, oldrte, false, true, true);
+            addNSItemtoQuery(pstate, oldnsitem, false, true, true);
             break;
         case CMD_UPDATE:
-            addRTEtoQuery(pstate, oldrte, false, true, true);
-            addRTEtoQuery(pstate, newrte, false, true, true);
+            addNSItemtoQuery(pstate, oldnsitem, false, true, true);
+            addNSItemtoQuery(pstate, newnsitem, false, true, true);
             break;
         case CMD_INSERT:
-            addRTEtoQuery(pstate, newrte, false, true, true);
+            addNSItemtoQuery(pstate, newnsitem, false, true, true);
             break;
         case CMD_DELETE:
-            addRTEtoQuery(pstate, oldrte, false, true, true);
+            addNSItemtoQuery(pstate, oldnsitem, false, true, true);
             break;
         default:
             elog(ERROR, "unrecognized event type: %d",
@@ -2832,18 +2832,18 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
              * nor "*" in the rule actions.  We decide later whether to put
              * them in the joinlist.
              */
-            oldrte = addRangeTableEntryForRelation(sub_pstate, rel,
-                                                   AccessShareLock,
-                                                   makeAlias("old", NIL),
-                                                   false, false);
-            newrte = addRangeTableEntryForRelation(sub_pstate, rel,
-                                                   AccessShareLock,
-                                                   makeAlias("new", NIL),
-                                                   false, false);
-            oldrte->requiredPerms = 0;
-            newrte->requiredPerms = 0;
-            addRTEtoQuery(sub_pstate, oldrte, false, true, false);
-            addRTEtoQuery(sub_pstate, newrte, false, true, false);
+            oldnsitem = addRangeTableEntryForRelation(sub_pstate, rel,
+                                                      AccessShareLock,
+                                                      makeAlias("old", NIL),
+                                                      false, false);
+            newnsitem = addRangeTableEntryForRelation(sub_pstate, rel,
+                                                      AccessShareLock,
+                                                      makeAlias("new", NIL),
+                                                      false, false);
+            oldnsitem->p_rte->requiredPerms = 0;
+            newnsitem->p_rte->requiredPerms = 0;
+            addNSItemtoQuery(sub_pstate, oldnsitem, false, true, false);
+            addNSItemtoQuery(sub_pstate, newnsitem, false, true, false);

             /* Transform the rule action statement */
             top_subqry = transformStmt(sub_pstate,
@@ -2967,6 +2967,8 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
              */
             if (has_old || (has_new && stmt->event == CMD_UPDATE))
             {
+                RangeTblRef *rtr;
+
                 /*
                  * If sub_qry is a setop, manipulating its jointree will do no
                  * good at all, because the jointree is dummy. (This should be
@@ -2976,11 +2978,11 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
                     ereport(ERROR,
                             (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                              errmsg("conditional UNION/INTERSECT/EXCEPT statements are not implemented")));
-                /* hack so we can use addRTEtoQuery() */
-                sub_pstate->p_rtable = sub_qry->rtable;
-                sub_pstate->p_joinlist = sub_qry->jointree->fromlist;
-                addRTEtoQuery(sub_pstate, oldrte, true, false, false);
-                sub_qry->jointree->fromlist = sub_pstate->p_joinlist;
+                /* hackishly add OLD to the already-built FROM clause */
+                rtr = makeNode(RangeTblRef);
+                rtr->rtindex = oldnsitem->p_rtindex;
+                sub_qry->jointree->fromlist =
+                    lappend(sub_qry->jointree->fromlist, rtr);
             }

             newactions = lappend(newactions, top_subqry);
@@ -3025,7 +3027,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
     List       *newcmds = NIL;
     bool        skipValidation = true;
     AlterTableCmd *newcmd;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;

     /*
      * We must not scribble on the passed-in AlterTableStmt, so copy it. (This
@@ -3040,13 +3042,13 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
     /* Set up pstate */
     pstate = make_parsestate(NULL);
     pstate->p_sourcetext = queryString;
-    rte = addRangeTableEntryForRelation(pstate,
-                                        rel,
-                                        AccessShareLock,
-                                        NULL,
-                                        false,
-                                        true);
-    addRTEtoQuery(pstate, rte, false, true, true);
+    nsitem = addRangeTableEntryForRelation(pstate,
+                                           rel,
+                                           AccessShareLock,
+                                           NULL,
+                                           false,
+                                           true);
+    addNSItemtoQuery(pstate, nsitem, false, true, true);

     /* Set up CreateStmtContext */
     cxt.pstate = pstate;
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index e01d18c..8f4eb8b 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -777,8 +777,8 @@ copy_table(Relation rel)
     copybuf = makeStringInfo();

     pstate = make_parsestate(NULL);
-    addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
-                                  NULL, false, false);
+    (void) addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
+                                         NULL, false, false);

     attnamelist = make_copy_attnamelist(relmapentry);
     cstate = BeginCopyFrom(pstate, rel, NULL, false, copy_read_data, attnamelist, NIL);
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 334efba..54fa806 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -3227,6 +3227,7 @@ rewriteTargetView(Query *parsetree, Relation view)
     {
         Index        old_exclRelIndex,
                     new_exclRelIndex;
+        ParseNamespaceItem *new_exclNSItem;
         RangeTblEntry *new_exclRte;
         List       *tmp_tlist;

@@ -3261,11 +3262,12 @@ rewriteTargetView(Query *parsetree, Relation view)
          */
         old_exclRelIndex = parsetree->onConflict->exclRelIndex;

-        new_exclRte = addRangeTableEntryForRelation(make_parsestate(NULL),
-                                                    base_rel,
-                                                    RowExclusiveLock,
-                                                    makeAlias("excluded", NIL),
-                                                    false, false);
+        new_exclNSItem = addRangeTableEntryForRelation(make_parsestate(NULL),
+                                                       base_rel,
+                                                       RowExclusiveLock,
+                                                       makeAlias("excluded", NIL),
+                                                       false, false);
+        new_exclRte = new_exclNSItem->p_rte;
         new_exclRte->relkind = RELKIND_COMPOSITE_TYPE;
         new_exclRte->requiredPerms = 0;
         /* other permissions fields in new_exclRte are already empty */
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 674acc5..93e7c09 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -19,6 +19,11 @@
 #include "utils/relcache.h"


+/* Forward references for some structs declared below */
+typedef struct ParseState ParseState;
+typedef struct ParseNamespaceItem ParseNamespaceItem;
+typedef struct ParseNamespaceColumn ParseNamespaceColumn;
+
 /*
  * Expression kinds distinguished by transformExpr().  Many of these are not
  * semantically distinct so far as expression transformation goes; rather,
@@ -79,8 +84,6 @@ typedef enum ParseExprKind
 /*
  * Function signatures for parser hooks
  */
-typedef struct ParseState ParseState;
-
 typedef Node *(*PreParseColumnRefHook) (ParseState *pstate, ColumnRef *cref);
 typedef Node *(*PostParseColumnRefHook) (ParseState *pstate, ColumnRef *cref, Node *var);
 typedef Node *(*ParseParamRefHook) (ParseState *pstate, ParamRef *pref);
@@ -132,9 +135,7 @@ typedef Node *(*CoerceParamHook) (ParseState *pstate, Param *param,
  *
  * p_target_relation: target relation, if query is INSERT, UPDATE, or DELETE.
  *
- * p_target_rangetblentry: target relation's entry in the rtable list.
- *
- * p_target_rtindex: target relation's index in the rtable list.
+ * p_target_nsitem: target relation's ParseNamespaceItem.
  *
  * p_is_insert: true to process assignment expressions like INSERT, false
  * to process them like UPDATE.  (Note this can change intra-statement, for
@@ -174,7 +175,7 @@ typedef Node *(*CoerceParamHook) (ParseState *pstate, Param *param,
  */
 struct ParseState
 {
-    struct ParseState *parentParseState;    /* stack link */
+    ParseState *parentParseState;    /* stack link */
     const char *p_sourcetext;    /* source text, or NULL if not available */
     List       *p_rtable;        /* range table so far */
     List       *p_joinexprs;    /* JoinExprs for RTE_JOIN p_rtable entries */
@@ -187,8 +188,7 @@ struct ParseState
     List       *p_future_ctes;    /* common table exprs not yet in namespace */
     CommonTableExpr *p_parent_cte;    /* this query's containing CTE */
     Relation    p_target_relation;    /* INSERT/UPDATE/DELETE target rel */
-    RangeTblEntry *p_target_rangetblentry;    /* target rel's RTE, or NULL */
-    int            p_target_rtindex;    /* target rel's RT index, or 0 */
+    ParseNamespaceItem *p_target_nsitem;    /* target rel's NSItem, or NULL */
     bool        p_is_insert;    /* process assignment like INSERT not UPDATE */
     List       *p_windowdefs;    /* raw representations of window clauses */
     ParseExprKind p_expr_kind;    /* what kind of expression we're parsing */
@@ -225,6 +225,10 @@ struct ParseState
 /*
  * An element of a namespace list.
  *
+ * The p_nscolumns array contains info showing how to construct Vars
+ * referencing corresponding elements of the RTE's colnames list.  If the RTE
+ * contains dropped columns, the corresponding array elements are all-zeroes.
+ *
  * Namespace items with p_rel_visible set define which RTEs are accessible by
  * qualified names, while those with p_cols_visible set define which RTEs are
  * accessible by unqualified names.  These sets are different because a JOIN
@@ -249,15 +253,46 @@ struct ParseState
  * are more complicated than "must have different alias names", so in practice
  * code searching a namespace list has to check for ambiguous references.
  */
-typedef struct ParseNamespaceItem
+struct ParseNamespaceItem
 {
     RangeTblEntry *p_rte;        /* The relation's rangetable entry */
     int            p_rtindex;        /* The relation's index in the rangetable */
+    /* array of same length as p_rte->eref->colnames: */
+    ParseNamespaceColumn *p_nscolumns;    /* per-column data */
     bool        p_rel_visible;    /* Relation name is visible? */
     bool        p_cols_visible; /* Column names visible as unqualified refs? */
     bool        p_lateral_only; /* Is only visible to LATERAL expressions? */
     bool        p_lateral_ok;    /* If so, does join type allow use? */
-} ParseNamespaceItem;
+};
+
+/*
+ * Data about one column of a ParseNamespaceItem.
+ *
+ * We track the info needed to construct a Var referencing the column
+ * (but only for user-defined columns; system column references and
+ * whole-row references are handled separately).
+ *
+ * p_varno and p_varattno identify the semantic referent, which is a
+ * base-relation column unless the reference is to a join USING column that
+ * isn't semantically equivalent to either join input column (because it is a
+ * FULL join or the input column requires a type coercion).  In those cases
+ * p_varno and p_varattno refer to the JOIN RTE.
+ *
+ * p_varnosyn and p_varattnosyn are either identical to p_varno/p_varattno,
+ * or they specify the column's position in an aliased JOIN RTE that hides
+ * the semantic referent RTE's refname.  (That could be either the JOIN RTE
+ * in which this ParseNamespaceColumn entry exists, or some lower join level.)
+ */
+struct ParseNamespaceColumn
+{
+    Index        p_varno;        /* rangetable index */
+    AttrNumber    p_varattno;        /* attribute number of the column */
+    Oid            p_vartype;        /* pg_type OID */
+    int32        p_vartypmod;    /* type modifier value */
+    Oid            p_varcollid;    /* OID of collation, or InvalidOid */
+    Index        p_varnosyn;        /* rangetable index of syntactic referent */
+    AttrNumber    p_varattnosyn;    /* attribute number of syntactic referent */
+};

 /* Support for parser_errposition_callback function */
 typedef struct ParseCallbackState
diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
index b09a71e..cb44fd4 100644
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -45,66 +45,70 @@ extern void markVarForSelectPriv(ParseState *pstate, Var *var,
                                  RangeTblEntry *rte);
 extern Relation parserOpenTable(ParseState *pstate, const RangeVar *relation,
                                 int lockmode);
-extern RangeTblEntry *addRangeTableEntry(ParseState *pstate,
-                                         RangeVar *relation,
-                                         Alias *alias,
-                                         bool inh,
-                                         bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForRelation(ParseState *pstate,
-                                                    Relation rel,
-                                                    int lockmode,
-                                                    Alias *alias,
-                                                    bool inh,
-                                                    bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForSubquery(ParseState *pstate,
-                                                    Query *subquery,
-                                                    Alias *alias,
-                                                    bool lateral,
-                                                    bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForFunction(ParseState *pstate,
-                                                    List *funcnames,
-                                                    List *funcexprs,
-                                                    List *coldeflists,
-                                                    RangeFunction *rangefunc,
-                                                    bool lateral,
-                                                    bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForValues(ParseState *pstate,
-                                                  List *exprs,
-                                                  List *coltypes,
-                                                  List *coltypmods,
-                                                  List *colcollations,
-                                                  Alias *alias,
-                                                  bool lateral,
-                                                  bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForTableFunc(ParseState *pstate,
-                                                     TableFunc *tf,
+extern ParseNamespaceItem *addRangeTableEntry(ParseState *pstate,
+                                              RangeVar *relation,
+                                              Alias *alias,
+                                              bool inh,
+                                              bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForRelation(ParseState *pstate,
+                                                         Relation rel,
+                                                         int lockmode,
+                                                         Alias *alias,
+                                                         bool inh,
+                                                         bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForSubquery(ParseState *pstate,
+                                                         Query *subquery,
+                                                         Alias *alias,
+                                                         bool lateral,
+                                                         bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForFunction(ParseState *pstate,
+                                                         List *funcnames,
+                                                         List *funcexprs,
+                                                         List *coldeflists,
+                                                         RangeFunction *rangefunc,
+                                                         bool lateral,
+                                                         bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForValues(ParseState *pstate,
+                                                       List *exprs,
+                                                       List *coltypes,
+                                                       List *coltypmods,
+                                                       List *colcollations,
+                                                       Alias *alias,
+                                                       bool lateral,
+                                                       bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForTableFunc(ParseState *pstate,
+                                                          TableFunc *tf,
+                                                          Alias *alias,
+                                                          bool lateral,
+                                                          bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForJoin(ParseState *pstate,
+                                                     List *colnames,
+                                                     ParseNamespaceColumn *nscolumns,
+                                                     JoinType jointype,
+                                                     List *aliasvars,
                                                      Alias *alias,
-                                                     bool lateral,
                                                      bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForJoin(ParseState *pstate,
-                                                List *colnames,
-                                                JoinType jointype,
-                                                List *aliasvars,
-                                                Alias *alias,
-                                                bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForCTE(ParseState *pstate,
-                                               CommonTableExpr *cte,
-                                               Index levelsup,
-                                               RangeVar *rv,
-                                               bool inFromCl);
-extern RangeTblEntry *addRangeTableEntryForENR(ParseState *pstate,
-                                               RangeVar *rv,
-                                               bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForCTE(ParseState *pstate,
+                                                    CommonTableExpr *cte,
+                                                    Index levelsup,
+                                                    RangeVar *rv,
+                                                    bool inFromCl);
+extern ParseNamespaceItem *addRangeTableEntryForENR(ParseState *pstate,
+                                                    RangeVar *rv,
+                                                    bool inFromCl);
 extern bool isLockedRefname(ParseState *pstate, const char *refname);
-extern void addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,
-                          bool addToJoinList,
-                          bool addToRelNameSpace, bool addToVarNameSpace);
+extern void addNSItemtoQuery(ParseState *pstate, ParseNamespaceItem *nsitem,
+                             bool addToJoinList,
+                             bool addToRelNameSpace, bool addToVarNameSpace);
 extern void errorMissingRTE(ParseState *pstate, RangeVar *relation) pg_attribute_noreturn();
 extern void errorMissingColumn(ParseState *pstate,
                                const char *relname, const char *colname, int location) pg_attribute_noreturn();
 extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                       int location, bool include_dropped,
                       List **colnames, List **colvars);
+extern List *expandNSItemVars(ParseNamespaceItem *nsitem,
+                              int sublevels_up, int location,
+                              List **colnames);
 extern List *expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
                                int sublevels_up, int location);
 extern int    attnameAttNum(Relation rd, const char *attname, bool sysColOK);
diff --git a/src/include/parser/parsetree.h b/src/include/parser/parsetree.h
index cf47e10..a2c3832 100644
--- a/src/include/parser/parsetree.h
+++ b/src/include/parser/parsetree.h
@@ -38,15 +38,7 @@
 extern char *get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum);

 /*
- * Given an RTE and an attribute number, return the appropriate
- * type and typemod info for that attribute of that RTE.
- */
-extern void get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
-                                   Oid *vartype, int32 *vartypmod, Oid *varcollid);
-
-/*
- * Check whether an attribute of an RTE has been dropped (note that
- * get_rte_attribute_type will fail on such an attr)
+ * Check whether an attribute of an RTE has been dropped
  */
 extern bool get_rte_attribute_is_dropped(RangeTblEntry *rte,
                                          AttrNumber attnum);
diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c
index f576ecb..66df39d 100644
--- a/src/test/modules/test_rls_hooks/test_rls_hooks.c
+++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c
@@ -70,7 +70,7 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation)
     Node       *e;
     ColumnRef  *c;
     ParseState *qual_pstate;
-    RangeTblEntry *rte;
+    ParseNamespaceItem *nsitem;

     if (strcmp(RelationGetRelationName(relation), "rls_test_permissive") != 0 &&
         strcmp(RelationGetRelationName(relation), "rls_test_both") != 0)
@@ -78,9 +78,10 @@ test_rls_hooks_permissive(CmdType cmdtype, Relation relation)

     qual_pstate = make_parsestate(NULL);

-    rte = addRangeTableEntryForRelation(qual_pstate, relation, AccessShareLock,
-                                        NULL, false, false);
-    addRTEtoQuery(qual_pstate, rte, false, true, true);
+    nsitem = addRangeTableEntryForRelation(qual_pstate,
+                                           relation, AccessShareLock,
+                                           NULL, false, false);
+    addNSItemtoQuery(qual_pstate, nsitem, false, true, true);

     role = ObjectIdGetDatum(ACL_ID_PUBLIC);

@@ -134,8 +135,7 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation)
     Node       *e;
     ColumnRef  *c;
     ParseState *qual_pstate;
-    RangeTblEntry *rte;
-
+    ParseNamespaceItem *nsitem;

     if (strcmp(RelationGetRelationName(relation), "rls_test_restrictive") != 0 &&
         strcmp(RelationGetRelationName(relation), "rls_test_both") != 0)
@@ -143,9 +143,10 @@ test_rls_hooks_restrictive(CmdType cmdtype, Relation relation)

     qual_pstate = make_parsestate(NULL);

-    rte = addRangeTableEntryForRelation(qual_pstate, relation, AccessShareLock,
-                                        NULL, false, false);
-    addRTEtoQuery(qual_pstate, rte, false, true, true);
+    nsitem = addRangeTableEntryForRelation(qual_pstate,
+                                           relation, AccessShareLock,
+                                           NULL, false, false);
+    addNSItemtoQuery(qual_pstate, nsitem, false, true, true);

     role = ObjectIdGetDatum(ACL_ID_PUBLIC);


Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
I wrote:
> Here is a further step on this journey.  It's still just parser
> refactoring, and doesn't (AFAICT) result in any change in generated
> parse trees, but it seems worth posting and committing separately.

Pushed at 5815696bc.

> 2. Add per-column information to the ParseNamespaceItems.  As of
> this patch, the useful part of that is column type/typmod/collation
> info which can be used to generate Vars referencing this RTE.
> I envision that the next step will involve generating the Vars'
> identity (varno/varattno columns) from that as well, and this
> patch includes logic to set up some associated per-column fields.
> But those are not actually getting fed into the Vars quite yet.

Here's a further step that does that.

The core idea of this patch is to make the parser generate join alias
Vars (that is, ones with varno pointing to a JOIN RTE) only when the
alias Var is actually different from any raw join input, that is a type
coercion and/or COALESCE is necessary to generate the join output value.
Otherwise just generate varno/varattno pointing to the relevant join
input column.

In effect, this means that the planner's flatten_join_alias_vars()
transformation is already done in the parser, for all cases except
(a) columns that are merged by JOIN USING and are transformed in the
process, and (b) whole-row join Vars.  In principle that would allow
us to skip doing flatten_join_alias_vars() in many more queries than
we do now, but we don't have quite enough infrastructure to know that
we can do so --- in particular there's no cheap way to know whether
there are any whole-row join Vars.  I'm not sure if it's worth the
trouble to add a Query-level flag for that, and in any case it seems
like fit material for a separate patch.  But even without skipping the
work entirely, this should make flatten_join_alias_vars() faster,
particularly where there are nested joins that it had to flatten
recursively.

An essential part of this change is to replace Var nodes'
varnoold/varoattno fields with varnosyn/varattnosyn, which have
considerably more tightly-defined meanings than the old fields: when
they differ from varno/varattno, they identify the Var's position in
an aliased JOIN RTE, and the join alias is what ruleutils.c should
print for the Var.  This is necessary because the varno change
destroyed ruleutils.c's ability to find the JOIN RTE from the Var's
varno.

Another way in which this change broke ruleutils.c is that it's no
longer feasible to determine, from a JOIN RTE's joinaliasvars list,
which join columns correspond to which columns of the join's immediate
input relations.  (If those are sub-joins, the joinaliasvars entries
may point to columns of their base relations, not the sub-joins.)
But that was a horrid mess requiring a lot of fragile assumptions
already, so let's just bite the bullet and add some more JOIN RTE
fields to make it more straightforward to figure that out.  I added
two integer-List fields containing the relevant column numbers from
the left and right input rels, plus a count of how many merged columns
there are.

This patch depends on the ParseNamespaceColumn infrastructure that
I added in commit 5815696bc.  The biggest bit of code change is
restructuring transformFromClauseItem's handling of JOINs so that
the ParseNamespaceColumn data is propagated upward correctly.

Other than that and the ruleutils fixes, everything pretty much
just works, though some processing is now inessential.  I grabbed
two pieces of low-hanging fruit in that line:

1. In find_expr_references, we don't need to recurse into join alias
Vars anymore.  There aren't any except for references to merged USING
columns, which are more properly handled when we scan the join's RTE.
This change actually fixes an edge-case issue: we will now record a
dependency on any type-coercion function present in a USING column's
joinaliasvar, even if that join column has no references in the query
text.  The odds of the missing dependency causing a problem seem quite
small: you'd have to posit somebody dropping an implicit cast between
two data types, without removing the types themselves, and then having
a stored rule containing a whole-row Var for a join whose USING merge
depends on that cast.  So I don't feel a great need to change this in
the back branches.  But in theory this way is more correct.

2. markRTEForSelectPriv and markTargetListOrigin don't need to recurse
into join alias Vars either, because the cases they care about don't
apply to alias Vars for USING columns that are semantically distinct
from the underlying columns.  This removes the only case in which
markVarForSelectPriv could be called with NULL for the RTE, so adjust
the comments to describe that hack as being strictly internal to
markRTEForSelectPriv.

            regards, tom lane

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 4c9ad49..c4a4df2 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1718,12 +1718,6 @@ recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
 /*
  * Recursively search an expression tree for object references.
  *
- * Note: we avoid creating references to columns of tables that participate
- * in an SQL JOIN construct, but are not actually used anywhere in the query.
- * To do so, we do not scan the joinaliasvars list of a join RTE while
- * scanning the query rangetable, but instead scan each individual entry
- * of the alias list when we find a reference to it.
- *
  * Note: in many cases we do not need to create dependencies on the datatypes
  * involved in an expression, because we'll have an indirect dependency via
  * some other object.  For instance Var nodes depend on a column which depends
@@ -1773,24 +1767,15 @@ find_expr_references_walker(Node *node,
             add_object_address(OCLASS_CLASS, rte->relid, var->varattno,
                                context->addrs);
         }
-        else if (rte->rtekind == RTE_JOIN)
-        {
-            /* Scan join output column to add references to join inputs */
-            List       *save_rtables;
-
-            /* We must make the context appropriate for join's level */
-            save_rtables = context->rtables;
-            context->rtables = list_copy_tail(context->rtables,
-                                              var->varlevelsup);
-            if (var->varattno <= 0 ||
-                var->varattno > list_length(rte->joinaliasvars))
-                elog(ERROR, "invalid varattno %d", var->varattno);
-            find_expr_references_walker((Node *) list_nth(rte->joinaliasvars,
-                                                          var->varattno - 1),
-                                        context);
-            list_free(context->rtables);
-            context->rtables = save_rtables;
-        }
+
+        /*
+         * Vars referencing other RTE types require no additional work.  In
+         * particular, a join alias Var can be ignored, because it must
+         * reference a merged USING column.  The relevant join input columns
+         * will also be referenced in the join qual, and any type coercion
+         * functions involved in the alias expression will be dealt with when
+         * we scan the RTE itself.
+         */
         return false;
     }
     else if (IsA(node, Const))
@@ -2147,11 +2132,13 @@ find_expr_references_walker(Node *node,

         /*
          * Add whole-relation refs for each plain relation mentioned in the
-         * subquery's rtable.
+         * subquery's rtable, and ensure we add refs for any type-coercion
+         * functions used in join alias lists.
          *
          * Note: query_tree_walker takes care of recursing into RTE_FUNCTION
-         * RTEs, subqueries, etc, so no need to do that here.  But keep it
-         * from looking at join alias lists.
+         * RTEs, subqueries, etc, so no need to do that here.  But we must
+         * tell it not to visit join alias lists, or we'll add refs for join
+         * input columns whether or not they are actually used in our query.
          *
          * Note: we don't need to worry about collations mentioned in
          * RTE_VALUES or RTE_CTE RTEs, because those must just duplicate
@@ -2169,6 +2156,31 @@ find_expr_references_walker(Node *node,
                     add_object_address(OCLASS_CLASS, rte->relid, 0,
                                        context->addrs);
                     break;
+                case RTE_JOIN:
+
+                    /*
+                     * Examine joinaliasvars entries only for merged JOIN
+                     * USING columns.  Only those entries could contain
+                     * type-coercion functions.  Also, their join input
+                     * columns must be referenced in the join quals, so this
+                     * won't accidentally add refs to otherwise-unused join
+                     * input columns.  (We want to ref the type coercion
+                     * functions even if the merged column isn't explicitly
+                     * used anywhere, to protect possible expansion of the
+                     * join RTE as a whole-row var, and because it seems like
+                     * a bad idea to allow dropping a function that's present
+                     * in our query tree, whether or not it could get called.)
+                     */
+                    context->rtables = lcons(query->rtable, context->rtables);
+                    for (int i = 0; i < rte->joinmergedcols; i++)
+                    {
+                        Node       *aliasvar = list_nth(rte->joinaliasvars, i);
+
+                        if (!IsA(aliasvar, Var))
+                            find_expr_references_walker(aliasvar, context);
+                    }
+                    context->rtables = list_delete_first(context->rtables);
+                    break;
                 default:
                     break;
             }
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 8034d5a..54ad62b 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1367,8 +1367,8 @@ _copyVar(const Var *from)
     COPY_SCALAR_FIELD(vartypmod);
     COPY_SCALAR_FIELD(varcollid);
     COPY_SCALAR_FIELD(varlevelsup);
-    COPY_SCALAR_FIELD(varnoold);
-    COPY_SCALAR_FIELD(varoattno);
+    COPY_SCALAR_FIELD(varnosyn);
+    COPY_SCALAR_FIELD(varattnosyn);
     COPY_LOCATION_FIELD(location);

     return newnode;
@@ -2373,7 +2373,10 @@ _copyRangeTblEntry(const RangeTblEntry *from)
     COPY_NODE_FIELD(subquery);
     COPY_SCALAR_FIELD(security_barrier);
     COPY_SCALAR_FIELD(jointype);
+    COPY_SCALAR_FIELD(joinmergedcols);
     COPY_NODE_FIELD(joinaliasvars);
+    COPY_NODE_FIELD(joinleftcols);
+    COPY_NODE_FIELD(joinrightcols);
     COPY_NODE_FIELD(functions);
     COPY_SCALAR_FIELD(funcordinality);
     COPY_NODE_FIELD(tablefunc);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 9c8070c..5b1ba14 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -168,8 +168,12 @@ _equalVar(const Var *a, const Var *b)
     COMPARE_SCALAR_FIELD(vartypmod);
     COMPARE_SCALAR_FIELD(varcollid);
     COMPARE_SCALAR_FIELD(varlevelsup);
-    COMPARE_SCALAR_FIELD(varnoold);
-    COMPARE_SCALAR_FIELD(varoattno);
+
+    /*
+     * varnosyn/varattnosyn are intentionally ignored here, because Vars with
+     * different syntactic identifiers are semantically the same as long as
+     * their varno/varattno match.
+     */
     COMPARE_LOCATION_FIELD(location);

     return true;
@@ -2657,7 +2661,10 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
     COMPARE_NODE_FIELD(subquery);
     COMPARE_SCALAR_FIELD(security_barrier);
     COMPARE_SCALAR_FIELD(jointype);
+    COMPARE_SCALAR_FIELD(joinmergedcols);
     COMPARE_NODE_FIELD(joinaliasvars);
+    COMPARE_NODE_FIELD(joinleftcols);
+    COMPARE_NODE_FIELD(joinrightcols);
     COMPARE_NODE_FIELD(functions);
     COMPARE_SCALAR_FIELD(funcordinality);
     COMPARE_NODE_FIELD(tablefunc);
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index f7d63df..e8cdc90 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -80,13 +80,13 @@ makeVar(Index varno,
     var->varlevelsup = varlevelsup;

     /*
-     * Since few if any routines ever create Var nodes with varnoold/varoattno
-     * different from varno/varattno, we don't provide separate arguments for
-     * them, but just initialize them to the given varno/varattno. This
+     * Only a few callers need to make Var nodes with varnosyn/varattnosyn
+     * different from varno/varattno.  We don't provide separate arguments for
+     * them, but just initialize them to the given varno/varattno.  This
      * reduces code clutter and chance of error for most callers.
      */
-    var->varnoold = varno;
-    var->varoattno = varattno;
+    var->varnosyn = varno;
+    var->varattnosyn = varattno;

     /* Likewise, we just set location to "unknown" here */
     var->location = -1;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index a53d473..d76fae4 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1074,8 +1074,8 @@ _outVar(StringInfo str, const Var *node)
     WRITE_INT_FIELD(vartypmod);
     WRITE_OID_FIELD(varcollid);
     WRITE_UINT_FIELD(varlevelsup);
-    WRITE_UINT_FIELD(varnoold);
-    WRITE_INT_FIELD(varoattno);
+    WRITE_UINT_FIELD(varnosyn);
+    WRITE_INT_FIELD(varattnosyn);
     WRITE_LOCATION_FIELD(location);
 }

@@ -3071,7 +3071,10 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
             break;
         case RTE_JOIN:
             WRITE_ENUM_FIELD(jointype, JoinType);
+            WRITE_INT_FIELD(joinmergedcols);
             WRITE_NODE_FIELD(joinaliasvars);
+            WRITE_NODE_FIELD(joinleftcols);
+            WRITE_NODE_FIELD(joinrightcols);
             break;
         case RTE_FUNCTION:
             WRITE_NODE_FIELD(functions);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 81e7b94..551ce6c 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -540,8 +540,8 @@ _readVar(void)
     READ_INT_FIELD(vartypmod);
     READ_OID_FIELD(varcollid);
     READ_UINT_FIELD(varlevelsup);
-    READ_UINT_FIELD(varnoold);
-    READ_INT_FIELD(varoattno);
+    READ_UINT_FIELD(varnosyn);
+    READ_INT_FIELD(varattnosyn);
     READ_LOCATION_FIELD(location);

     READ_DONE();
@@ -1400,7 +1400,10 @@ _readRangeTblEntry(void)
             break;
         case RTE_JOIN:
             READ_ENUM_FIELD(jointype, JoinType);
+            READ_INT_FIELD(joinmergedcols);
             READ_NODE_FIELD(joinaliasvars);
+            READ_NODE_FIELD(joinleftcols);
+            READ_NODE_FIELD(joinrightcols);
             break;
         case RTE_FUNCTION:
             READ_NODE_FIELD(functions);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index ebb0a59..3dcded5 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -428,6 +428,8 @@ add_rte_to_flat_rtable(PlannerGlobal *glob, RangeTblEntry *rte)
     newrte->tablesample = NULL;
     newrte->subquery = NULL;
     newrte->joinaliasvars = NIL;
+    newrte->joinleftcols = NIL;
+    newrte->joinrightcols = NIL;
     newrte->functions = NIL;
     newrte->tablefunc = NULL;
     newrte->values_lists = NIL;
@@ -1681,8 +1683,8 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
         Assert(var->varno != OUTER_VAR);
         if (!IS_SPECIAL_VARNO(var->varno))
             var->varno += context->rtoffset;
-        if (var->varnoold > 0)
-            var->varnoold += context->rtoffset;
+        if (var->varnosyn > 0)
+            var->varnosyn += context->rtoffset;
         return (Node *) var;
     }
     if (IsA(node, Param))
@@ -2110,15 +2112,16 @@ set_dummy_tlist_references(Plan *plan, int rtoffset)
                          exprTypmod((Node *) oldvar),
                          exprCollation((Node *) oldvar),
                          0);
-        if (IsA(oldvar, Var))
+        if (IsA(oldvar, Var) &&
+            oldvar->varnosyn > 0)
         {
-            newvar->varnoold = oldvar->varno + rtoffset;
-            newvar->varoattno = oldvar->varattno;
+            newvar->varnosyn = oldvar->varnosyn + rtoffset;
+            newvar->varattnosyn = oldvar->varattnosyn;
         }
         else
         {
-            newvar->varnoold = 0;    /* wasn't ever a plain Var */
-            newvar->varoattno = 0;
+            newvar->varnosyn = 0;    /* wasn't ever a plain Var */
+            newvar->varattnosyn = 0;
         }

         tle = flatCopyTargetEntry(tle);
@@ -2242,7 +2245,7 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
  *
  * If a match is found, return a copy of the given Var with suitably
  * modified varno/varattno (to wit, newvarno and the resno of the TLE entry).
- * Also ensure that varnoold is incremented by rtoffset.
+ * Also ensure that varnosyn is incremented by rtoffset.
  * If no match, return NULL.
  */
 static Var *
@@ -2265,8 +2268,8 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,

             newvar->varno = newvarno;
             newvar->varattno = vinfo->resno;
-            if (newvar->varnoold > 0)
-                newvar->varnoold += rtoffset;
+            if (newvar->varnosyn > 0)
+                newvar->varnosyn += rtoffset;
             return newvar;
         }
         vinfo++;
@@ -2308,8 +2311,8 @@ search_indexed_tlist_for_non_var(Expr *node,
         Var           *newvar;

         newvar = makeVarFromTargetEntry(newvarno, tle);
-        newvar->varnoold = 0;    /* wasn't ever a plain Var */
-        newvar->varoattno = 0;
+        newvar->varnosyn = 0;    /* wasn't ever a plain Var */
+        newvar->varattnosyn = 0;
         return newvar;
     }
     return NULL;                /* no match */
@@ -2345,8 +2348,8 @@ search_indexed_tlist_for_sortgroupref(Expr *node,
             Var           *newvar;

             newvar = makeVarFromTargetEntry(newvarno, tle);
-            newvar->varnoold = 0;    /* wasn't ever a plain Var */
-            newvar->varoattno = 0;
+            newvar->varnosyn = 0;    /* wasn't ever a plain Var */
+            newvar->varattnosyn = 0;
             return newvar;
         }
     }
@@ -2384,7 +2387,7 @@ search_indexed_tlist_for_sortgroupref(Expr *node,
  *        or NULL
  * 'acceptable_rel' is either zero or the rangetable index of a relation
  *        whose Vars may appear in the clause without provoking an error
- * 'rtoffset': how much to increment varnoold by
+ * 'rtoffset': how much to increment varnos by
  *
  * Returns the new expression tree.  The original clause structure is
  * not modified.
@@ -2445,8 +2448,8 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
         {
             var = copyVar(var);
             var->varno += context->rtoffset;
-            if (var->varnoold > 0)
-                var->varnoold += context->rtoffset;
+            if (var->varnosyn > 0)
+                var->varnosyn += context->rtoffset;
             return (Node *) var;
         }

@@ -2528,7 +2531,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
  * 'node': the tree to be fixed (a target item or qual)
  * 'subplan_itlist': indexed target list for subplan (or index)
  * 'newvarno': varno to use for Vars referencing tlist elements
- * 'rtoffset': how much to increment varnoold by
+ * 'rtoffset': how much to increment varnos by
  *
  * The resulting tree is a copy of the original in which all Var nodes have
  * varno = newvarno, varattno = resno of corresponding targetlist element.
diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index ad50865..d722063 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -255,6 +255,9 @@ adjust_appendrel_attrs_mutator(Node *node,
         Var           *var = (Var *) copyObject(node);
         AppendRelInfo *appinfo = NULL;

+        if (var->varlevelsup != 0)
+            return (Node *) var;    /* no changes needed */
+
         for (cnt = 0; cnt < nappinfos; cnt++)
         {
             if (var->varno == appinfos[cnt]->parent_relid)
@@ -264,10 +267,12 @@ adjust_appendrel_attrs_mutator(Node *node,
             }
         }

-        if (var->varlevelsup == 0 && appinfo)
+        if (appinfo)
         {
             var->varno = appinfo->child_relid;
-            var->varnoold = appinfo->child_relid;
+            /* it's now a generated Var, so drop any syntactic labeling */
+            var->varnosyn = 0;
+            var->varattnosyn = 0;
             if (var->varattno > 0)
             {
                 Node       *newnode;
diff --git a/src/backend/optimizer/util/paramassign.c b/src/backend/optimizer/util/paramassign.c
index 45f992c..93fae07 100644
--- a/src/backend/optimizer/util/paramassign.c
+++ b/src/backend/optimizer/util/paramassign.c
@@ -83,15 +83,14 @@ assign_param_for_var(PlannerInfo *root, Var *var)

             /*
              * This comparison must match _equalVar(), except for ignoring
-             * varlevelsup.  Note that _equalVar() ignores the location.
+             * varlevelsup.  Note that _equalVar() ignores varnosyn,
+             * varattnosyn, and location, so this does too.
              */
             if (pvar->varno == var->varno &&
                 pvar->varattno == var->varattno &&
                 pvar->vartype == var->vartype &&
                 pvar->vartypmod == var->vartypmod &&
-                pvar->varcollid == var->varcollid &&
-                pvar->varnoold == var->varnoold &&
-                pvar->varoattno == var->varoattno)
+                pvar->varcollid == var->varcollid)
                 return pitem->paramId;
         }
     }
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 447a61e..748bebf 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1734,7 +1734,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
                                         targetnames,
                                         sortnscolumns,
                                         JOIN_INNER,
+                                        0,
                                         targetvars,
+                                        NIL,
+                                        NIL,
                                         NULL,
                                         false);

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 5fa42d3..36a3eff 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -52,9 +52,9 @@
 #include "utils/syscache.h"


-static void extractRemainingColumns(List *common_colnames,
-                                    List *src_colnames, List *src_colvars,
-                                    ParseNamespaceColumn *src_nscolumns,
+static int    extractRemainingColumns(ParseNamespaceColumn *src_nscolumns,
+                                    List *src_colnames,
+                                    List **src_colnos,
                                     List **res_colnames, List **res_colvars,
                                     ParseNamespaceColumn *res_nscolumns);
 static Node *transformJoinUsingClause(ParseState *pstate,
@@ -76,6 +76,7 @@ static ParseNamespaceItem *getNSItemForSpecialRelationTypes(ParseState *pstate,
 static Node *transformFromClauseItem(ParseState *pstate, Node *n,
                                      ParseNamespaceItem **top_nsitem,
                                      List **namespace);
+static Var *buildVarFromNSColumn(ParseNamespaceColumn *nscol);
 static Node *buildMergedJoinVar(ParseState *pstate, JoinType jointype,
                                 Var *l_colvar, Var *r_colvar);
 static void setNamespaceColumnVisibility(List *namespace, bool cols_visible);
@@ -237,64 +238,61 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
 /*
  * Extract all not-in-common columns from column lists of a source table
  *
- * We hand back new lists in *res_colnames and *res_colvars.  But
- * res_nscolumns points to a caller-allocated array that we fill in
- * the next few entries in.
+ * src_nscolumns and src_colnames describe the source table.
+ *
+ * *src_colnos initially contains the column numbers of the already-merged
+ * columns.  We add to it the column number of each additional column.
+ * Also append to *res_colnames the name of each additional column,
+ * append to *res_colvars a Var for each additional column, and copy the
+ * columns' nscolumns data into res_nscolumns[] (which is caller-allocated
+ * space that had better be big enough).
+ *
+ * Returns the number of columns added.
  */
-static void
-extractRemainingColumns(List *common_colnames,
-                        List *src_colnames, List *src_colvars,
-                        ParseNamespaceColumn *src_nscolumns,
+static int
+extractRemainingColumns(ParseNamespaceColumn *src_nscolumns,
+                        List *src_colnames,
+                        List **src_colnos,
                         List **res_colnames, List **res_colvars,
                         ParseNamespaceColumn *res_nscolumns)
 {
-    List       *new_colnames = NIL;
-    List       *new_colvars = NIL;
-    ListCell   *lnames,
-               *lvars;
-
-    Assert(list_length(src_colnames) == list_length(src_colvars));
+    int            colcount = 0;
+    Bitmapset  *prevcols;
+    int            attnum;
+    ListCell   *lc;

-    forboth(lnames, src_colnames, lvars, src_colvars)
+    /*
+     * While we could just test "list_member_int(*src_colnos, attnum)" to
+     * detect already-merged columns in the loop below, that would be O(N^2)
+     * for a wide input table.  Instead build a bitmapset of just the merged
+     * USING columns, which we won't add to within the main loop.
+     */
+    prevcols = NULL;
+    foreach(lc, *src_colnos)
     {
-        char       *colname = strVal(lfirst(lnames));
-        bool        match = false;
-        ListCell   *cnames;
-
-        /*
-         * Ignore any dropped columns in the src_nscolumns input.  (The list
-         * inputs won't contain entries for dropped columns.)
-         */
-        while (src_nscolumns->p_varno == 0)
-            src_nscolumns++;
-
-        /* is current src column already accounted for in common_colnames? */
-        foreach(cnames, common_colnames)
-        {
-            char       *ccolname = strVal(lfirst(cnames));
+        prevcols = bms_add_member(prevcols, lfirst_int(lc));
+    }

-            if (strcmp(colname, ccolname) == 0)
-            {
-                match = true;
-                break;
-            }
-        }
+    attnum = 0;
+    foreach(lc, src_colnames)
+    {
+        char       *colname = strVal(lfirst(lc));

-        if (!match)
+        attnum++;
+        /* Non-dropped and not already merged? */
+        if (colname[0] != '\0' && !bms_is_member(attnum, prevcols))
         {
-            /* Nope, so emit it as next output column */
-            new_colnames = lappend(new_colnames, lfirst(lnames));
-            new_colvars = lappend(new_colvars, lfirst(lvars));
+            /* Yes, so emit it as next output column */
+            *src_colnos = lappend_int(*src_colnos, attnum);
+            *res_colnames = lappend(*res_colnames, lfirst(lc));
+            *res_colvars = lappend(*res_colvars,
+                                   buildVarFromNSColumn(src_nscolumns + attnum - 1));
             /* Copy the input relation's nscolumn data for this column */
-            *res_nscolumns = *src_nscolumns;
-            res_nscolumns++;
+            res_nscolumns[colcount] = src_nscolumns[attnum - 1];
+            colcount++;
         }
-
-        src_nscolumns++;
     }
-
-    *res_colnames = new_colnames;
-    *res_colvars = new_colvars;
+    return colcount;
 }

 /* transformJoinUsingClause()
@@ -1154,10 +1152,12 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                    *l_colnames,
                    *r_colnames,
                    *res_colnames,
-                   *l_colvars,
-                   *r_colvars,
+                   *l_colnos,
+                   *r_colnos,
                    *res_colvars;
-        ParseNamespaceColumn *res_nscolumns;
+        ParseNamespaceColumn *l_nscolumns,
+                   *r_nscolumns,
+                   *res_nscolumns;
         int            res_colindex;
         bool        lateral_ok;
         int            sv_namespace_length;
@@ -1211,12 +1211,15 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         my_namespace = list_concat(l_namespace, r_namespace);

         /*
-         * Extract column name and var lists from both subtrees
-         *
-         * Note: expandNSItemVars returns new lists, safe for me to modify
+         * We'll work from the nscolumns data and eref alias column names for
+         * each of the input nsitems.  Note that these include dropped
+         * columns, which is helpful because we can keep track of physical
+         * input column numbers more easily.
          */
-        l_colvars = expandNSItemVars(l_nsitem, 0, -1, &l_colnames);
-        r_colvars = expandNSItemVars(r_nsitem, 0, -1, &r_colnames);
+        l_nscolumns = l_nsitem->p_nscolumns;
+        l_colnames = l_nsitem->p_rte->eref->colnames;
+        r_nscolumns = r_nsitem->p_nscolumns;
+        r_colnames = r_nsitem->p_rte->eref->colnames;

         /*
          * Natural join does not explicitly specify columns; must generate
@@ -1240,6 +1243,9 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                 char       *l_colname = strVal(lfirst(lx));
                 Value       *m_name = NULL;

+                if (l_colname[0] == '\0')
+                    continue;    /* ignore dropped columns */
+
                 foreach(rx, r_colnames)
                 {
                     char       *r_colname = strVal(lfirst(rx));
@@ -1262,6 +1268,8 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         /*
          * Now transform the join qualifications, if any.
          */
+        l_colnos = NIL;
+        r_colnos = NIL;
         res_colnames = NIL;
         res_colvars = NIL;

@@ -1297,6 +1305,8 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                 Node       *u_colvar;
                 ParseNamespaceColumn *res_nscolumn;

+                Assert(u_colname[0] != '\0');
+
                 /* Check for USING(foo,foo) */
                 foreach(col, res_colnames)
                 {
@@ -1331,6 +1341,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                             (errcode(ERRCODE_UNDEFINED_COLUMN),
                              errmsg("column \"%s\" specified in USING clause does not exist in left table",
                                     u_colname)));
+                l_colnos = lappend_int(l_colnos, l_index + 1);

                 /* Find it in right input */
                 ndx = 0;
@@ -1354,10 +1365,11 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                             (errcode(ERRCODE_UNDEFINED_COLUMN),
                              errmsg("column \"%s\" specified in USING clause does not exist in right table",
                                     u_colname)));
+                r_colnos = lappend_int(r_colnos, r_index + 1);

-                l_colvar = list_nth(l_colvars, l_index);
+                l_colvar = buildVarFromNSColumn(l_nscolumns + l_index);
                 l_usingvars = lappend(l_usingvars, l_colvar);
-                r_colvar = list_nth(r_colvars, r_index);
+                r_colvar = buildVarFromNSColumn(r_nscolumns + r_index);
                 r_usingvars = lappend(r_usingvars, r_colvar);

                 res_colnames = lappend(res_colnames, lfirst(ucol));
@@ -1371,26 +1383,12 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                 if (u_colvar == (Node *) l_colvar)
                 {
                     /* Merged column is equivalent to left input */
-                    res_nscolumn->p_varno = l_colvar->varno;
-                    res_nscolumn->p_varattno = l_colvar->varattno;
-                    res_nscolumn->p_vartype = l_colvar->vartype;
-                    res_nscolumn->p_vartypmod = l_colvar->vartypmod;
-                    res_nscolumn->p_varcollid = l_colvar->varcollid;
-                    /* XXX these are not quite right, but doesn't matter yet */
-                    res_nscolumn->p_varnosyn = l_colvar->varno;
-                    res_nscolumn->p_varattnosyn = l_colvar->varattno;
+                    *res_nscolumn = l_nscolumns[l_index];
                 }
                 else if (u_colvar == (Node *) r_colvar)
                 {
                     /* Merged column is equivalent to right input */
-                    res_nscolumn->p_varno = r_colvar->varno;
-                    res_nscolumn->p_varattno = r_colvar->varattno;
-                    res_nscolumn->p_vartype = r_colvar->vartype;
-                    res_nscolumn->p_vartypmod = r_colvar->vartypmod;
-                    res_nscolumn->p_varcollid = r_colvar->varcollid;
-                    /* XXX these are not quite right, but doesn't matter yet */
-                    res_nscolumn->p_varnosyn = r_colvar->varno;
-                    res_nscolumn->p_varattnosyn = r_colvar->varattno;
+                    *res_nscolumn = r_nscolumns[r_index];
                 }
                 else
                 {
@@ -1427,22 +1425,14 @@ transformFromClauseItem(ParseState *pstate, Node *n,
         }

         /* Add remaining columns from each side to the output columns */
-        extractRemainingColumns(res_colnames,
-                                l_colnames, l_colvars,
-                                l_nsitem->p_nscolumns,
-                                &l_colnames, &l_colvars,
-                                res_nscolumns + res_colindex);
-        res_colindex += list_length(l_colvars);
-        extractRemainingColumns(res_colnames,
-                                r_colnames, r_colvars,
-                                r_nsitem->p_nscolumns,
-                                &r_colnames, &r_colvars,
-                                res_nscolumns + res_colindex);
-        res_colindex += list_length(r_colvars);
-        res_colnames = list_concat(res_colnames, l_colnames);
-        res_colvars = list_concat(res_colvars, l_colvars);
-        res_colnames = list_concat(res_colnames, r_colnames);
-        res_colvars = list_concat(res_colvars, r_colvars);
+        res_colindex +=
+            extractRemainingColumns(l_nscolumns, l_colnames, &l_colnos,
+                                    &res_colnames, &res_colvars,
+                                    res_nscolumns + res_colindex);
+        res_colindex +=
+            extractRemainingColumns(r_nscolumns, r_colnames, &r_colnos,
+                                    &res_colnames, &res_colvars,
+                                    res_nscolumns + res_colindex);

         /*
          * Check alias (AS clause), if any.
@@ -1468,7 +1458,10 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                                            res_colnames,
                                            res_nscolumns,
                                            j->jointype,
+                                           list_length(j->usingClause),
                                            res_colvars,
+                                           l_colnos,
+                                           r_colnos,
                                            j->alias,
                                            true);

@@ -1538,6 +1531,30 @@ transformFromClauseItem(ParseState *pstate, Node *n,
 }

 /*
+ * buildVarFromNSColumn -
+ *      build a Var node using ParseNamespaceColumn data
+ *
+ * We assume varlevelsup should be 0, and no location is specified
+ */
+static Var *
+buildVarFromNSColumn(ParseNamespaceColumn *nscol)
+{
+    Var           *var;
+
+    Assert(nscol->p_varno > 0); /* i.e., not deleted column */
+    var = makeVar(nscol->p_varno,
+                  nscol->p_varattno,
+                  nscol->p_vartype,
+                  nscol->p_vartypmod,
+                  nscol->p_varcollid,
+                  0);
+    /* makeVar doesn't offer parameters for these, so set by hand: */
+    var->varnosyn = nscol->p_varnosyn;
+    var->varattnosyn = nscol->p_varattnosyn;
+    return var;
+}
+
+/*
  * buildMergedJoinVar -
  *      generate a suitable replacement expression for a merged join column
  */
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index d7c4bae..abfac9d 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -714,12 +714,15 @@ scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
                             colname,
                             rte->eref->aliasname)));

-        var = makeVar(nsitem->p_rtindex,
-                      attnum,
+        var = makeVar(nscol->p_varno,
+                      nscol->p_varattno,
                       nscol->p_vartype,
                       nscol->p_vartypmod,
                       nscol->p_varcollid,
                       sublevels_up);
+        /* makeVar doesn't offer parameters for these, so set them by hand: */
+        var->varnosyn = nscol->p_varnosyn;
+        var->varattnosyn = nscol->p_varattnosyn;
     }
     else
     {
@@ -991,9 +994,10 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colnam
  *
  * col == InvalidAttrNumber means a "whole row" reference
  *
- * The caller should pass the actual RTE if it has it handy; otherwise pass
- * NULL, and we'll look it up here.  (This uglification of the API is
- * worthwhile because nearly all external callers have the RTE at hand.)
+ * External callers should always pass the Var's RTE.  Internally, we
+ * allow NULL to be passed for the RTE and then look it up if needed;
+ * this takes less code than requiring each internal recursion site
+ * to perform a lookup.
  */
 static void
 markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
@@ -1062,21 +1066,11 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
         else
         {
             /*
-             * Regular join attribute, look at the alias-variable list.
-             *
-             * The aliasvar could be either a Var or a COALESCE expression,
-             * but in the latter case we should already have marked the two
-             * referent variables as being selected, due to their use in the
-             * JOIN clause.  So we need only be concerned with the Var case.
-             * But we do need to drill down through implicit coercions.
+             * Join alias Vars for ordinary columns must refer to merged JOIN
+             * USING columns.  We don't need to do anything here, because the
+             * join input columns will also be referenced in the join's qual
+             * clause, and will get marked for select privilege there.
              */
-            Var           *aliasvar;
-
-            Assert(col > 0 && col <= list_length(rte->joinaliasvars));
-            aliasvar = (Var *) list_nth(rte->joinaliasvars, col - 1);
-            aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar);
-            if (aliasvar && IsA(aliasvar, Var))
-                markVarForSelectPriv(pstate, aliasvar, NULL);
         }
     }
     /* other RTE types don't require privilege marking */
@@ -1085,9 +1079,6 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
 /*
  * markVarForSelectPriv
  *       Mark the RTE referenced by a Var as requiring SELECT privilege
- *
- * The caller should pass the Var's referenced RTE if it has it handy
- * (nearly all do); otherwise pass NULL.
  */
 void
 markVarForSelectPriv(ParseState *pstate, Var *var, RangeTblEntry *rte)
@@ -2104,7 +2095,10 @@ addRangeTableEntryForJoin(ParseState *pstate,
                           List *colnames,
                           ParseNamespaceColumn *nscolumns,
                           JoinType jointype,
+                          int nummergedcols,
                           List *aliasvars,
+                          List *leftcols,
+                          List *rightcols,
                           Alias *alias,
                           bool inFromCl)
 {
@@ -2129,7 +2123,10 @@ addRangeTableEntryForJoin(ParseState *pstate,
     rte->relid = InvalidOid;
     rte->subquery = NULL;
     rte->jointype = jointype;
+    rte->joinmergedcols = nummergedcols;
     rte->joinaliasvars = aliasvars;
+    rte->joinleftcols = leftcols;
+    rte->joinrightcols = rightcols;
     rte->alias = alias;

     eref = alias ? copyObject(alias) : makeAlias("unnamed_join", NIL);
@@ -2706,11 +2703,11 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,

                     /*
                      * During ordinary parsing, there will never be any
-                     * deleted columns in the join; but we have to check since
-                     * this routine is also used by the rewriter, and joins
-                     * found in stored rules might have join columns for
-                     * since-deleted columns.  This will be signaled by a null
-                     * pointer in the alias-vars list.
+                     * deleted columns in the join.  While this function is
+                     * also used by the rewriter and planner, they do not
+                     * currently call it on any JOIN RTEs.  Therefore, this
+                     * next bit is dead code, but it seems prudent to handle
+                     * the case correctly anyway.
                      */
                     if (avar == NULL)
                     {
@@ -2746,11 +2743,26 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
                     {
                         Var           *varnode;

-                        varnode = makeVar(rtindex, varattno,
-                                          exprType(avar),
-                                          exprTypmod(avar),
-                                          exprCollation(avar),
-                                          sublevels_up);
+                        /*
+                         * If the joinaliasvars entry is a simple Var, just
+                         * copy it (with adjustment of varlevelsup and
+                         * location); otherwise it is a JOIN USING column and
+                         * we must generate a join alias Var.  This matches
+                         * the results that expansion of "join.*" by
+                         * expandNSItemVars would have produced, if we had
+                         * access to the ParseNamespaceItem for the join.
+                         */
+                        if (IsA(avar, Var))
+                        {
+                            varnode = copyObject((Var *) avar);
+                            varnode->varlevelsup = sublevels_up;
+                        }
+                        else
+                            varnode = makeVar(rtindex, varattno,
+                                              exprType(avar),
+                                              exprTypmod(avar),
+                                              exprCollation(avar),
+                                              sublevels_up);
                         varnode->location = location;

                         *colvars = lappend(*colvars, varnode);
@@ -2964,12 +2976,15 @@ expandNSItemVars(ParseNamespaceItem *nsitem,
             Var           *var;

             Assert(nscol->p_varno > 0);
-            var = makeVar(nsitem->p_rtindex,
-                          colindex + 1,
+            var = makeVar(nscol->p_varno,
+                          nscol->p_varattno,
                           nscol->p_vartype,
                           nscol->p_vartypmod,
                           nscol->p_varcollid,
                           sublevels_up);
+            /* makeVar doesn't offer parameters for these, so set by hand: */
+            var->varnosyn = nscol->p_varnosyn;
+            var->varattnosyn = nscol->p_varattnosyn;
             var->location = location;
             result = lappend(result, var);
             if (colnames)
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 8476d3c..566c517 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -346,8 +346,11 @@ markTargetListOrigins(ParseState *pstate, List *targetlist)
  *
  * levelsup is an extra offset to interpret the Var's varlevelsup correctly.
  *
- * This is split out so it can recurse for join references.  Note that we
- * do not drill down into views, but report the view as the column owner.
+ * Note that we do not drill down into views, but report the view as the
+ * column owner.  There's also no need to drill down into joins: if we see
+ * a join alias Var, it must be a merged JOIN USING column (or possibly a
+ * whole-row Var); that is not a direct reference to any plain table column,
+ * so we don't report it.
  */
 static void
 markTargetListOrigin(ParseState *pstate, TargetEntry *tle,
@@ -385,17 +388,6 @@ markTargetListOrigin(ParseState *pstate, TargetEntry *tle,
             }
             break;
         case RTE_JOIN:
-            /* Join RTE --- recursively inspect the alias variable */
-            if (attnum != InvalidAttrNumber)
-            {
-                Var           *aliasvar;
-
-                Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars));
-                aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1);
-                /* We intentionally don't strip implicit coercions here */
-                markTargetListOrigin(pstate, tle, aliasvar, netlevelsup);
-            }
-            break;
         case RTE_FUNCTION:
         case RTE_VALUES:
         case RTE_TABLEFUNC:
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 23ba9a1..a727f41 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -322,7 +322,7 @@ contains_multiexpr_param(Node *node, void *context)
  *
  * Find all Var nodes in the given tree with varlevelsup == sublevels_up,
  * and increment their varno fields (rangetable indexes) by 'offset'.
- * The varnoold fields are adjusted similarly.  Also, adjust other nodes
+ * The varnosyn fields are adjusted similarly.  Also, adjust other nodes
  * that contain rangetable indexes, such as RangeTblRef and JoinExpr.
  *
  * NOTE: although this has the form of a walker, we cheat and modify the
@@ -348,7 +348,8 @@ OffsetVarNodes_walker(Node *node, OffsetVarNodes_context *context)
         if (var->varlevelsup == context->sublevels_up)
         {
             var->varno += context->offset;
-            var->varnoold += context->offset;
+            if (var->varnosyn > 0)
+                var->varnosyn += context->offset;
         }
         return false;
     }
@@ -485,7 +486,7 @@ offset_relid_set(Relids relids, int offset)
  *
  * Find all Var nodes in the given tree belonging to a specific relation
  * (identified by sublevels_up and rt_index), and change their varno fields
- * to 'new_index'.  The varnoold fields are changed too.  Also, adjust other
+ * to 'new_index'.  The varnosyn fields are changed too.  Also, adjust other
  * nodes that contain rangetable indexes, such as RangeTblRef and JoinExpr.
  *
  * NOTE: although this has the form of a walker, we cheat and modify the
@@ -513,7 +514,9 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
             var->varno == context->rt_index)
         {
             var->varno = context->new_index;
-            var->varnoold = context->new_index;
+            /* If the syntactic referent is same RTE, fix it too */
+            if (var->varnosyn == context->rt_index)
+                var->varnosyn = context->new_index;
         }
         return false;
     }
@@ -1252,7 +1255,10 @@ map_variable_attnos_mutator(Node *node,
                     context->attno_map->attnums[attno - 1] == 0)
                     elog(ERROR, "unexpected varattno %d in expression to be mapped",
                          attno);
-                newvar->varattno = newvar->varoattno = context->attno_map->attnums[attno - 1];
+                newvar->varattno = context->attno_map->attnums[attno - 1];
+                /* If the syntactic referent is same RTE, fix it too */
+                if (newvar->varnosyn == context->target_varno)
+                    newvar->varattnosyn = newvar->varattno;
             }
             else if (attno == 0)
             {
@@ -1453,7 +1459,7 @@ ReplaceVarsFromTargetList_callback(Var *var,
             case REPLACEVARS_CHANGE_VARNO:
                 var = (Var *) copyObject(var);
                 var->varno = rcon->nomatch_varno;
-                var->varnoold = rcon->nomatch_varno;
+                /* we leave the syntactic referent alone */
                 return (Node *) var;

             case REPLACEVARS_SUBSTITUTE_NULL:
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 0018ffc..116e00b 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -271,7 +271,8 @@ typedef struct
      * child RTE's attno and rightattnos[i] is zero; and conversely for a
      * column of the right child.  But for merged columns produced by JOIN
      * USING/NATURAL JOIN, both leftattnos[i] and rightattnos[i] are nonzero.
-     * Also, if the column has been dropped, both are zero.
+     * Note that a simple reference might be to a child RTE column that's been
+     * dropped; but that's OK since the column could not be used in the query.
      *
      * If it's a JOIN USING, usingNames holds the alias names selected for the
      * merged columns (these might be different from the original USING list,
@@ -368,8 +369,6 @@ static char *make_colname_unique(char *colname, deparse_namespace *dpns,
 static void expand_colnames_array_to(deparse_columns *colinfo, int n);
 static void identify_join_columns(JoinExpr *j, RangeTblEntry *jrte,
                                   deparse_columns *colinfo);
-static void flatten_join_using_qual(Node *qual,
-                                    List **leftvars, List **rightvars);
 static char *get_rtable_name(int rtindex, deparse_context *context);
 static void set_deparse_plan(deparse_namespace *dpns, Plan *plan);
 static void push_child_plan(deparse_namespace *dpns, Plan *plan,
@@ -3722,13 +3721,13 @@ has_dangerous_join_using(deparse_namespace *dpns, Node *jtnode)
              * dangerous situation and must pick unique aliases.
              */
             RangeTblEntry *jrte = rt_fetch(j->rtindex, dpns->rtable);
-            ListCell   *lc;

-            foreach(lc, jrte->joinaliasvars)
+            /* We need only examine the merged columns */
+            for (int i = 0; i < jrte->joinmergedcols; i++)
             {
-                Var           *aliasvar = (Var *) lfirst(lc);
+                Node       *aliasvar = list_nth(jrte->joinaliasvars, i);

-                if (aliasvar != NULL && !IsA(aliasvar, Var))
+                if (!IsA(aliasvar, Var))
                     return true;
             }
         }
@@ -4137,12 +4136,8 @@ set_join_column_names(deparse_namespace *dpns, RangeTblEntry *rte,
         char       *colname = colinfo->colnames[i];
         char       *real_colname;

-        /* Ignore dropped column (only possible for non-merged column) */
-        if (colinfo->leftattnos[i] == 0 && colinfo->rightattnos[i] == 0)
-        {
-            Assert(colname == NULL);
-            continue;
-        }
+        /* Join column must refer to at least one input column */
+        Assert(colinfo->leftattnos[i] != 0 || colinfo->rightattnos[i] != 0);

         /* Get the child column name */
         if (colinfo->leftattnos[i] > 0)
@@ -4154,7 +4149,13 @@ set_join_column_names(deparse_namespace *dpns, RangeTblEntry *rte,
             /* We're joining system columns --- use eref name */
             real_colname = strVal(list_nth(rte->eref->colnames, i));
         }
-        Assert(real_colname != NULL);
+
+        /* If child col has been dropped, no need to assign a join colname */
+        if (real_colname == NULL)
+        {
+            colinfo->colnames[i] = NULL;
+            continue;
+        }

         /* In an unnamed join, just report child column names as-is */
         if (rte->alias == NULL)
@@ -4479,7 +4480,8 @@ identify_join_columns(JoinExpr *j, RangeTblEntry *jrte,
                       deparse_columns *colinfo)
 {
     int            numjoincols;
-    int            i;
+    int            jcolno;
+    int            rcolno;
     ListCell   *lc;

     /* Extract left/right child RT indexes */
@@ -4508,146 +4510,32 @@ identify_join_columns(JoinExpr *j, RangeTblEntry *jrte,
     colinfo->leftattnos = (int *) palloc0(numjoincols * sizeof(int));
     colinfo->rightattnos = (int *) palloc0(numjoincols * sizeof(int));

-    /* Scan the joinaliasvars list to identify simple column references */
-    i = 0;
-    foreach(lc, jrte->joinaliasvars)
-    {
-        Var           *aliasvar = (Var *) lfirst(lc);
-
-        /* get rid of any implicit coercion above the Var */
-        aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar);
-
-        if (aliasvar == NULL)
-        {
-            /* It's a dropped column; nothing to do here */
-        }
-        else if (IsA(aliasvar, Var))
-        {
-            Assert(aliasvar->varlevelsup == 0);
-            Assert(aliasvar->varattno != 0);
-            if (aliasvar->varno == colinfo->leftrti)
-                colinfo->leftattnos[i] = aliasvar->varattno;
-            else if (aliasvar->varno == colinfo->rightrti)
-                colinfo->rightattnos[i] = aliasvar->varattno;
-            else
-                elog(ERROR, "unexpected varno %d in JOIN RTE",
-                     aliasvar->varno);
-        }
-        else if (IsA(aliasvar, CoalesceExpr))
-        {
-            /*
-             * It's a merged column in FULL JOIN USING.  Ignore it for now and
-             * let the code below identify the merged columns.
-             */
-        }
-        else
-            elog(ERROR, "unrecognized node type in join alias vars: %d",
-                 (int) nodeTag(aliasvar));
-
-        i++;
-    }
-
     /*
-     * If there's a USING clause, deconstruct the join quals to identify the
-     * merged columns.  This is a tad painful but if we cannot rely on the
-     * column names, there is no other representation of which columns were
-     * joined by USING.  (Unless the join type is FULL, we can't tell from the
-     * joinaliasvars list which columns are merged.)  Note: we assume that the
-     * merged columns are the first output column(s) of the join.
+     * Deconstruct RTE's joinleftcols/joinrightcols into desired format.
+     * Recall that the column(s) merged due to USING are the first column(s)
+     * of the join output.  We need not do anything special while scanning
+     * joinleftcols, but while scanning joinrightcols we must distinguish
+     * merged from unmerged columns.
      */
-    if (j->usingClause)
-    {
-        List       *leftvars = NIL;
-        List       *rightvars = NIL;
-        ListCell   *lc2;
-
-        /* Extract left- and right-side Vars from the qual expression */
-        flatten_join_using_qual(j->quals, &leftvars, &rightvars);
-        Assert(list_length(leftvars) == list_length(j->usingClause));
-        Assert(list_length(rightvars) == list_length(j->usingClause));
-
-        /* Mark the output columns accordingly */
-        i = 0;
-        forboth(lc, leftvars, lc2, rightvars)
-        {
-            Var           *leftvar = (Var *) lfirst(lc);
-            Var           *rightvar = (Var *) lfirst(lc2);
-
-            Assert(leftvar->varlevelsup == 0);
-            Assert(leftvar->varattno != 0);
-            if (leftvar->varno != colinfo->leftrti)
-                elog(ERROR, "unexpected varno %d in JOIN USING qual",
-                     leftvar->varno);
-            colinfo->leftattnos[i] = leftvar->varattno;
-
-            Assert(rightvar->varlevelsup == 0);
-            Assert(rightvar->varattno != 0);
-            if (rightvar->varno != colinfo->rightrti)
-                elog(ERROR, "unexpected varno %d in JOIN USING qual",
-                     rightvar->varno);
-            colinfo->rightattnos[i] = rightvar->varattno;
-
-            i++;
-        }
-    }
-}
-
-/*
- * flatten_join_using_qual: extract Vars being joined from a JOIN/USING qual
- *
- * We assume that transformJoinUsingClause won't have produced anything except
- * AND nodes, equality operator nodes, and possibly implicit coercions, and
- * that the AND node inputs match left-to-right with the original USING list.
- *
- * Caller must initialize the result lists to NIL.
- */
-static void
-flatten_join_using_qual(Node *qual, List **leftvars, List **rightvars)
-{
-    if (IsA(qual, BoolExpr))
+    jcolno = 0;
+    foreach(lc, jrte->joinleftcols)
     {
-        /* Handle AND nodes by recursion */
-        BoolExpr   *b = (BoolExpr *) qual;
-        ListCell   *lc;
+        int            leftattno = lfirst_int(lc);

-        Assert(b->boolop == AND_EXPR);
-        foreach(lc, b->args)
-        {
-            flatten_join_using_qual((Node *) lfirst(lc),
-                                    leftvars, rightvars);
-        }
+        colinfo->leftattnos[jcolno++] = leftattno;
     }
-    else if (IsA(qual, OpExpr))
-    {
-        /* Otherwise we should have an equality operator */
-        OpExpr       *op = (OpExpr *) qual;
-        Var           *var;
-
-        if (list_length(op->args) != 2)
-            elog(ERROR, "unexpected unary operator in JOIN/USING qual");
-        /* Arguments should be Vars with perhaps implicit coercions */
-        var = (Var *) strip_implicit_coercions((Node *) linitial(op->args));
-        if (!IsA(var, Var))
-            elog(ERROR, "unexpected node type in JOIN/USING qual: %d",
-                 (int) nodeTag(var));
-        *leftvars = lappend(*leftvars, var);
-        var = (Var *) strip_implicit_coercions((Node *) lsecond(op->args));
-        if (!IsA(var, Var))
-            elog(ERROR, "unexpected node type in JOIN/USING qual: %d",
-                 (int) nodeTag(var));
-        *rightvars = lappend(*rightvars, var);
-    }
-    else
+    rcolno = 0;
+    foreach(lc, jrte->joinrightcols)
     {
-        /* Perhaps we have an implicit coercion to boolean? */
-        Node       *q = strip_implicit_coercions(qual);
+        int            rightattno = lfirst_int(lc);

-        if (q != qual)
-            flatten_join_using_qual(q, leftvars, rightvars);
+        if (rcolno < jrte->joinmergedcols)    /* merged column? */
+            colinfo->rightattnos[rcolno] = rightattno;
         else
-            elog(ERROR, "unexpected node type in JOIN/USING qual: %d",
-                 (int) nodeTag(qual));
+            colinfo->rightattnos[jcolno++] = rightattno;
+        rcolno++;
     }
+    Assert(jcolno == numjoincols);
 }

 /*
@@ -6717,6 +6605,8 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
     AttrNumber    attnum;
     int            netlevelsup;
     deparse_namespace *dpns;
+    Index        varno;
+    AttrNumber    varattno;
     deparse_columns *colinfo;
     char       *refname;
     char       *attname;
@@ -6730,16 +6620,32 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
                                           netlevelsup);

     /*
+     * If we have a syntactic referent for the Var, and we're working from a
+     * parse tree, prefer to use the syntactic referent.  Otherwise, fall back
+     * on the semantic referent.  (Forcing use of the semantic referent when
+     * printing plan trees is a design choice that's perhaps more motivated by
+     * backwards compatibility than anything else.  But it does have the
+     * advantage of making plans more explicit.)
+     */
+    if (var->varnosyn > 0 && dpns->plan == NULL)
+    {
+        varno = var->varnosyn;
+        varattno = var->varattnosyn;
+    }
+    else
+    {
+        varno = var->varno;
+        varattno = var->varattno;
+    }
+
+    /*
      * Try to find the relevant RTE in this rtable.  In a plan tree, it's
      * likely that varno is OUTER_VAR or INNER_VAR, in which case we must dig
      * down into the subplans, or INDEX_VAR, which is resolved similarly. Also
      * find the aliases previously assigned for this RTE.
      */
-    if (var->varno >= 1 && var->varno <= list_length(dpns->rtable))
+    if (varno >= 1 && varno <= list_length(dpns->rtable))
     {
-        Index        varno = var->varno;
-        AttrNumber    varattno = var->varattno;
-
         /*
          * We might have been asked to map child Vars to some parent relation.
          */
@@ -6962,7 +6868,8 @@ resolve_special_varno(Node *node, deparse_context *context,
                                           var->varlevelsup);

     /*
-     * If varno is special, recurse.
+     * If varno is special, recurse.  (Don't worry about varnosyn; if we're
+     * here, we already decided not to use that.)
      */
     if (var->varno == OUTER_VAR && dpns->outer_tlist)
     {
@@ -7054,6 +6961,8 @@ get_name_for_var_field(Var *var, int fieldno,
     AttrNumber    attnum;
     int            netlevelsup;
     deparse_namespace *dpns;
+    Index        varno;
+    AttrNumber    varattno;
     TupleDesc    tupleDesc;
     Node       *expr;

@@ -7114,6 +7023,22 @@ get_name_for_var_field(Var *var, int fieldno,
                                           netlevelsup);

     /*
+     * If we have a syntactic referent for the Var, and we're working from a
+     * parse tree, prefer to use the syntactic referent.  Otherwise, fall back
+     * on the semantic referent.  (See comments in get_variable().)
+     */
+    if (var->varnosyn > 0 && dpns->plan == NULL)
+    {
+        varno = var->varnosyn;
+        varattno = var->varattnosyn;
+    }
+    else
+    {
+        varno = var->varno;
+        varattno = var->varattno;
+    }
+
+    /*
      * Try to find the relevant RTE in this rtable.  In a plan tree, it's
      * likely that varno is OUTER_VAR or INNER_VAR, in which case we must dig
      * down into the subplans, or INDEX_VAR, which is resolved similarly.
@@ -7122,20 +7047,20 @@ get_name_for_var_field(Var *var, int fieldno,
      * about inheritance mapping: a child Var should have the same datatype as
      * its parent, and here we're really only interested in the Var's type.
      */
-    if (var->varno >= 1 && var->varno <= list_length(dpns->rtable))
+    if (varno >= 1 && varno <= list_length(dpns->rtable))
     {
-        rte = rt_fetch(var->varno, dpns->rtable);
-        attnum = var->varattno;
+        rte = rt_fetch(varno, dpns->rtable);
+        attnum = varattno;
     }
-    else if (var->varno == OUTER_VAR && dpns->outer_tlist)
+    else if (varno == OUTER_VAR && dpns->outer_tlist)
     {
         TargetEntry *tle;
         deparse_namespace save_dpns;
         const char *result;

-        tle = get_tle_by_resno(dpns->outer_tlist, var->varattno);
+        tle = get_tle_by_resno(dpns->outer_tlist, varattno);
         if (!tle)
-            elog(ERROR, "bogus varattno for OUTER_VAR var: %d", var->varattno);
+            elog(ERROR, "bogus varattno for OUTER_VAR var: %d", varattno);

         Assert(netlevelsup == 0);
         push_child_plan(dpns, dpns->outer_plan, &save_dpns);
@@ -7146,15 +7071,15 @@ get_name_for_var_field(Var *var, int fieldno,
         pop_child_plan(dpns, &save_dpns);
         return result;
     }
-    else if (var->varno == INNER_VAR && dpns->inner_tlist)
+    else if (varno == INNER_VAR && dpns->inner_tlist)
     {
         TargetEntry *tle;
         deparse_namespace save_dpns;
         const char *result;

-        tle = get_tle_by_resno(dpns->inner_tlist, var->varattno);
+        tle = get_tle_by_resno(dpns->inner_tlist, varattno);
         if (!tle)
-            elog(ERROR, "bogus varattno for INNER_VAR var: %d", var->varattno);
+            elog(ERROR, "bogus varattno for INNER_VAR var: %d", varattno);

         Assert(netlevelsup == 0);
         push_child_plan(dpns, dpns->inner_plan, &save_dpns);
@@ -7165,14 +7090,14 @@ get_name_for_var_field(Var *var, int fieldno,
         pop_child_plan(dpns, &save_dpns);
         return result;
     }
-    else if (var->varno == INDEX_VAR && dpns->index_tlist)
+    else if (varno == INDEX_VAR && dpns->index_tlist)
     {
         TargetEntry *tle;
         const char *result;

-        tle = get_tle_by_resno(dpns->index_tlist, var->varattno);
+        tle = get_tle_by_resno(dpns->index_tlist, varattno);
         if (!tle)
-            elog(ERROR, "bogus varattno for INDEX_VAR var: %d", var->varattno);
+            elog(ERROR, "bogus varattno for INDEX_VAR var: %d", varattno);

         Assert(netlevelsup == 0);

@@ -7183,7 +7108,7 @@ get_name_for_var_field(Var *var, int fieldno,
     }
     else
     {
-        elog(ERROR, "bogus varno: %d", var->varno);
+        elog(ERROR, "bogus varno: %d", varno);
         return NULL;            /* keep compiler quiet */
     }

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f67bd9f..cdfa056 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1020,14 +1020,35 @@ typedef struct RangeTblEntry
      * be a Var of one of the join's input relations, or such a Var with an
      * implicit coercion to the join's output column type, or a COALESCE
      * expression containing the two input column Vars (possibly coerced).
-     * Within a Query loaded from a stored rule, it is also possible for
+     * Elements beyond the first joinmergedcols entries are always just Vars,
+     * and are never referenced from elsewhere in the query (that is, join
+     * alias Vars are generated only for merged columns).  We keep these
+     * entries only because they're needed in expandRTE() and similar code.
+     *
+     * Within a Query loaded from a stored rule, it is possible for non-merged
      * joinaliasvars items to be null pointers, which are placeholders for
      * (necessarily unreferenced) columns dropped since the rule was made.
      * Also, once planning begins, joinaliasvars items can be almost anything,
      * as a result of subquery-flattening substitutions.
+     *
+     * joinleftcols is an integer list of physical column numbers of the left
+     * join input rel that are included in the join; likewise joinrighttcols
+     * for the right join input rel.  (Which rels those are can be determined
+     * from the associated JoinExpr.)  If the join is USING/NATURAL, then the
+     * first joinmergedcols entries in each list identify the merged columns.
+     * The merged columns come first in the join output, then remaining
+     * columns of the left input, then remaining columns of the right.
+     *
+     * Note that input columns could have been dropped after creation of a
+     * stored rule, if they are not referenced in the query (in particular,
+     * merged columns could not be dropped); this is not accounted for in
+     * joinleftcols/joinrighttcols.
      */
     JoinType    jointype;        /* type of join */
+    int            joinmergedcols; /* number of merged (JOIN USING) columns */
     List       *joinaliasvars;    /* list of alias-var expansions */
+    List       *joinleftcols;    /* left-side input column numbers */
+    List       *joinrightcols;    /* right-side input column numbers */

     /*
      * Fields valid for a function RTE (else NIL/zero):
@@ -3313,8 +3334,8 @@ typedef struct ConstraintsSetStmt
  */

 /* Reindex options */
-#define REINDEXOPT_VERBOSE (1 << 0)    /* print progress info */
-#define REINDEXOPT_REPORT_PROGRESS (1 << 1)    /* report pgstat progress */
+#define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
+#define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */

 typedef enum ReindexObjectType
 {
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index eb2cacb..d73be2a 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -141,18 +141,32 @@ typedef struct Expr
 /*
  * Var - expression node representing a variable (ie, a table column)
  *
- * Note: during parsing/planning, varnoold/varoattno are always just copies
- * of varno/varattno.  At the tail end of planning, Var nodes appearing in
- * upper-level plan nodes are reassigned to point to the outputs of their
- * subplans; for example, in a join node varno becomes INNER_VAR or OUTER_VAR
- * and varattno becomes the index of the proper element of that subplan's
- * target list.  Similarly, INDEX_VAR is used to identify Vars that reference
- * an index column rather than a heap column.  (In ForeignScan and CustomScan
- * plan nodes, INDEX_VAR is abused to signify references to columns of a
- * custom scan tuple type.)  In all these cases, varnoold/varoattno hold the
- * original values.  The code doesn't really need varnoold/varoattno, but they
- * are very useful for debugging and interpreting completed plans, so we keep
- * them around.
+ * In the parser and planner, varno and varattno identify the semantic
+ * referent, which is a base-relation column unless the reference is to a join
+ * USING column that isn't semantically equivalent to either join input column
+ * (because it is a FULL join or the input column requires a type coercion).
+ * In those cases varno and varattno refer to the JOIN RTE.  (Early in the
+ * planner, we replace such join references by the implied expression; but up
+ * till then we want join reference Vars to keep their original identity for
+ * query-printing purposes.)
+ *
+ * At the end of planning, Var nodes appearing in upper-level plan nodes are
+ * reassigned to point to the outputs of their subplans; for example, in a
+ * join node varno becomes INNER_VAR or OUTER_VAR and varattno becomes the
+ * index of the proper element of that subplan's target list.  Similarly,
+ * INDEX_VAR is used to identify Vars that reference an index column rather
+ * than a heap column.  (In ForeignScan and CustomScan plan nodes, INDEX_VAR
+ * is abused to signify references to columns of a custom scan tuple type.)
+ *
+ * In the parser, varnosyn and varattnosyn are either identical to
+ * varno/varattno, or they specify the column's position in an aliased JOIN
+ * RTE that hides the semantic referent RTE's refname.  This is a syntactic
+ * identifier as opposed to the semantic identifier; it tells ruleutils.c
+ * how to print the Var properly.  varnosyn/varattnosyn retain their values
+ * throughout planning and execution, so they are particularly helpful to
+ * identify Vars when debugging.  Note, however, that a Var that is generated
+ * in the planner and doesn't correspond to any simple relation column may
+ * have varnosyn = varattnosyn = 0.
  */
 #define    INNER_VAR        65000    /* reference to inner subplan */
 #define    OUTER_VAR        65001    /* reference to outer subplan */
@@ -177,8 +191,8 @@ typedef struct Var
     Index        varlevelsup;    /* for subquery variables referencing outer
                                  * relations; 0 in a normal var, >0 means N
                                  * levels up */
-    Index        varnoold;        /* original value of varno, for debugging */
-    AttrNumber    varoattno;        /* original value of varattno */
+    Index        varnosyn;        /* syntactic relation index (0 if unknown) */
+    AttrNumber    varattnosyn;    /* syntactic attribute number */
     int            location;        /* token location, or -1 if unknown */
 } Var;

diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
index b8bff23..93f9446 100644
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -85,7 +85,10 @@ extern ParseNamespaceItem *addRangeTableEntryForJoin(ParseState *pstate,
                                                      List *colnames,
                                                      ParseNamespaceColumn *nscolumns,
                                                      JoinType jointype,
+                                                     int nummergedcols,
                                                      List *aliasvars,
+                                                     List *leftcols,
+                                                     List *rightcols,
                                                      Alias *alias,
                                                      bool inFromCl);
 extern ParseNamespaceItem *addRangeTableEntryForCTE(ParseState *pstate,

Re: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno

От
Tom Lane
Дата:
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Dec 20, 2019 at 11:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The alternatives that seem plausible at this point are
>> (1) Create some sort of wrapper node indicating "the contents of this
>> expression might be replaced by NULL".  This is basically what the
>> planner's PlaceHolderVars do, so maybe we'd just be talking about
>> introducing those at some earlier stage.
>> ...

> I'm not sure which is better, either, although I would like to note in
> passing that the name PlaceHolderVar seems to me to be confusing and
> terrible. It took me years to understand it, and I've never been
> totally sure that I actually do. Why is it not called
> MightBeNullWrapper or something?

Here's a data dump about my further thoughts in this area.  I've concluded
that the "wrapper" approach is the right way to proceed, and that rather
than having the planner introduce the wrappers as happens now, we should
indeed have the parser introduce the wrappers from the get-go.  There are
a few arguments for that:

* Arguably, this is a question of decorating the parse tree with
information about query semantics.  I've always held that parse analysis
is what should introduce knowledge of semantics; the planner ought not be
reverse-engineering that.

* AFAICS, we would need an additional pass over the query tree in order to
do this in the planner.  There is no existing recursive tree-modification
pass that happens at an appropriate time.

* We can use the same type of wrapper node to solve the problems with
grouping-set expressions that were discussed in
https://www.postgresql.org/message-id/flat/7dbdcf5c-b5a6-ef89-4958-da212fe10176%40iki.fi
although I'd leave that for a follow-on patch rather than try to fix
it immediately.  Here again, it'd be better to introduce the wrappers
at parse time --- check_ungrouped_columns() is already detecting the
presence of grouping-expression references, so we could make it inject
wrappers around them at relatively little extra cost.

Per Robert's complaint above, these wrappers need better documentation,
and they should be called something other than PlaceHolderVar, even though
they're basically that (and hopefully will replace those completely).
I'm tentatively thinking of calling them "NullableVar", but am open to
better ideas.  And here is a proposed addition to optimizer/README to
explain why they exist.  I'm not quite satisfied with the explanation yet
--- in particular, if we don't need them at runtime, why do we need them
at parse time?  Any thoughts about how to explain this more solidly are
welcome.

----------

To simplify handling of some issues with outer joins, we use NullableVars,
which are produced by the parser and used by the planner, but do not
appear in finished plans.  A NullableVar is a wrapper around another
expression, decorated with a set of outer-join relids, and notionally
having the semantics

    CASE WHEN any-of-these-outer-joins-produced-a-null-extended-row
    THEN NULL
    ELSE contained-expression
    END

It's only notional, because no such calculation is ever done explicitly.
In a finished plan, the NullableVar construct is replaced by a plain Var
referencing an output column of the topmost mentioned outer join, while
the "contained expression" is the corresponding input to the bottommost
join.  Any forcing to null happens in the course of calculating the
outer join results.  (Because we don't ever have to do the calculation
explicitly, it's not necessary to distinguish which side of an outer join
got null-extended, which'd otherwise be essential information for FULL
JOIN cases.)

A NullableVar wrapper is placed around a Var referencing a column of the
nullable side of an outer join when that reference appears syntactically
above (outside) the outer join, but not when the reference is below the
outer join, such as within its ON clause.  References to the non-nullable
side of an outer join are never wrapped.  NullableVars mentioning multiple
join nodes arise from cases with nested outer joins.

It might seem that the NullableVar construct is unnecessary (and indeed,
we got by without it for many years).  In a join row that's null-extended
for lack of a matching nullable-side row, the only reasonable value to
impute to a Var of that side is NULL, no matter where you look in the
parse tree.  However there are pressing reasons to use NullableVars
anyway:

* NullableVars simplify reasoning about where to evaluate qual clauses.
Consider
    SELECT * FROM t1 LEFT JOIN t2 ON (t1.x = t2.y) WHERE foo(t2.z)
(Assume foo() is not strict, so that we can't reduce the left join to
a plain join.)  A naive implementation might try to push the foo(t2.z)
call down to the scan of t2, but that is not correct because (a) what
foo() should actually see for a null-extended join row is NULL, and
(b) if foo() returns false, we should suppress the t1 row from the join
altogether, not emit it with a null-extended t2 row.  On the other hand,
it *would* be correct (and desirable) to push the call down if the query
were
    SELECT * FROM t1 LEFT JOIN t2 ON (t1.x = t2.y AND foo(t2.z))
If the upper WHERE clause is represented as foo(NullableVar(t2.z)), then
we can recognize that the NullableVar construct must be evaluated above
the join, since it references the join's relid.  Meanwhile, a t2.z
reference within the ON clause receives no such decoration, so in the
second case foo(t2.z) can be seen to be safe to push down to the scan
level.  Thus we can solve the qual-placement problem in a simple and
general fashion.

* NullableVars simplify reasoning around EquivalenceClasses.  Given say
    SELECT * FROM t1 LEFT JOIN t2 ON (t1.x = t2.y) WHERE t1.x = 42
we would like to put t1.x and t2.y and 42 into the same EquivalenceClass
and then derive "t2.y = 42" to use as a restriction clause for the scan
of t2.  However, it'd be wrong to conclude that t2.y will always have
the value 42, or that it's equal to t1.x in every joined row.  The use
of NullableVar wrappers sidesteps this problem: we can put t2.y in the
EquivalenceClass, and we can derive all the equalities we want about it,
but they will not lead to conclusions that NullableVar(t2.y) is equal to
anything.

* NullableVars are necessary to avoid wrong results when flattening
sub-selects.  If t2 in the above example is a sub-select or view in which
the y output column is a constant, and we want to pull up that sub-select,
we cannot simply substitute that constant for every use of t2.y in the
outer query: a Const node will not produce "NULL" when that's needed.
But it does work if the t2.y Vars are wrapped in NullableVars.  The
NullableVar shows that the contained value might be replaced by a NULL,
and it carries enough information so that we can generate a plan tree in
which that replacement does happen when necessary (by evaluating the
Const below the outer join and making upper references to it be Vars).
Moreover, when pulling up the constant into portions of the parse tree
that are below the outer join, the right things also happen: those
references can validly become plain Consts.

In essence, these examples show that it's useful to treat references to
a column of the nullable side of an outer join as being semantically
distinct depending on whether they are "above" or "below" the outer join,
even though no distinction exists once the calculation of a particular
join output row is complete.

----------

As you might gather from that, I'm thinking of changing the planner
so that (at least for outer joins) the relid set for a join includes
the RTE number of the join node itself.  I haven't decided yet if
that should happen across-the-board or just in the areas where we
use relid sets to decide which qual expressions get evaluated where.

Some other exciting things that will happen:

* RestrictInfo.is_pushed_down will go away; as sketched above, the
presence of the outer join's relid in the qual's required_relids
(due to NullableVars' outer join relid sets getting added into that
by pull_varnos) will tell us whether the qual must be treated as
a join or filter qual for the current join level.

* I think a lot of hackery in distribute_qual_to_rels can go away,
such as the below_outer_join flag, and maybe check_outerjoin_delay.
All of that is basically trying to reverse-engineer the qual
placement semantics that the wrappers will make explicit.

* As sketched above, equivalence classes will no longer need to
treat outer-join equalities with suspicion, and I think the
reconsider_outer_join_clauses stuff goes away too.

* There's probably other hackery that can be simplified; I've not
gone looking in detail yet.

I've not written any actual code, but am close to being ready to.
One thing I'm still struggling with is how to continue to support
outer join "identity 3":

        3.      (A leftjoin B on (Pab)) leftjoin C on (Pbc)
                = A leftjoin (B leftjoin C on (Pbc)) on (Pab)

        Identity 3 only holds if predicate Pbc must fail for all-null B
        rows (that is, Pbc is strict for at least one column of B).

Per this sketch, if the query is initially written the first way, Pbc's
references to B Vars would have NullableVars indicating a dependence on
the A/B join, seemingly preventing Pbc from being pushed into the RHS of
that join per the identity.  But if the query is initially written the
second way, there will be no NullableVar wrappers in either predicate.
Maybe it's sufficient to strip the NullableVar wrappers once we've
detected the applicability of the identity.  (We'll need code for that
anyway, since outer-join strength reduction will create cases where
NullableVar wrappers need to go away.)

            regards, tom lane