Thread: Clarifying/rationalizing Vars' varno/varattno/varnoold/varoattno
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
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
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
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);
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
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
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
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);
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,
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