Reconsider the representation of join alias Vars.

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 previously 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 5815696b.  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.

catversion bump required due to changes in stored rules.

Discussion: https://postgr.es/m/7115.1577986646@sss.pgh.pa.us

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;
 			}

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);

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);

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;

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);

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);

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.

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;

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;
 		}
 	}

src/backend/parser/analyze.c

@@ -1734,7 +1734,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 										targetnames,
 										sortnscolumns,
 										JOIN_INNER,
+										0,
 										targetvars,
+										NIL,
+										NIL,
 										NULL,
 										false);
 

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)
@@ -2110,7 +2101,10 @@ addRangeTableEntryForJoin(ParseState *pstate,
 						  List *colnames,
 						  ParseNamespaceColumn *nscolumns,
 						  JoinType jointype,
+						  int nummergedcols,
 						  List *aliasvars,
+						  List *leftcols,
+						  List *rightcols,
 						  Alias *alias,
 						  bool inFromCl)
 {
@@ -2135,7 +2129,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);
@@ -2713,11 +2710,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)
 					{
@@ -2753,11 +2750,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);
@@ -2971,12 +2983,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)

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:

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:

src/include/catalog/catversion.h

@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202001061
+#define CATALOG_VERSION_NO	202001091
 
 #endif

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
 {

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;
 

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,