Merge branch 'REL_10_STABLE' into PGPRO10

Author: vbwagner

contrib/postgres_fdw/deparse.c

@@ -178,7 +178,7 @@ static void appendGroupByClause(List *tlist, deparse_expr_cxt *context);
 static void appendAggOrderBy(List *orderList, List *targetList,
 				 deparse_expr_cxt *context);
 static void appendFunctionName(Oid funcid, deparse_expr_cxt *context);
-static Node *deparseSortGroupClause(Index ref, List *tlist,
+static Node *deparseSortGroupClause(Index ref, List *tlist, bool force_colno,
 					   deparse_expr_cxt *context);
 
 /*
@@ -2853,7 +2853,7 @@ appendAggOrderBy(List *orderList, List *targetList, deparse_expr_cxt *context)
 		first = false;
 
 		sortexpr = deparseSortGroupClause(srt->tleSortGroupRef, targetList,
-										  context);
+										  false, context);
 		sortcoltype = exprType(sortexpr);
 		/* See whether operator is default < or > for datatype */
 		typentry = lookup_type_cache(sortcoltype,
@@ -2960,7 +2960,7 @@ appendGroupByClause(List *tlist, deparse_expr_cxt *context)
 			appendStringInfoString(buf, ", ");
 		first = false;
 
-		deparseSortGroupClause(grp->tleSortGroupRef, tlist, context);
+		deparseSortGroupClause(grp->tleSortGroupRef, tlist, true, context);
 	}
 }
 
@@ -3047,7 +3047,8 @@ appendFunctionName(Oid funcid, deparse_expr_cxt *context)
  * need not find it again.
  */
 static Node *
-deparseSortGroupClause(Index ref, List *tlist, deparse_expr_cxt *context)
+deparseSortGroupClause(Index ref, List *tlist, bool force_colno,
+					   deparse_expr_cxt *context)
 {
 	StringInfo	buf = context->buf;
 	TargetEntry *tle;
@@ -3056,7 +3057,13 @@ deparseSortGroupClause(Index ref, List *tlist, deparse_expr_cxt *context)
 	tle = get_sortgroupref_tle(ref, tlist);
 	expr = tle->expr;
 
-	if (expr && IsA(expr, Const))
+	if (force_colno)
+	{
+		/* Use column-number form when requested by caller. */
+		Assert(!tle->resjunk);
+		appendStringInfo(buf, "%d", tle->resno);
+	}
+	else if (expr && IsA(expr, Const))
 	{
 		/*
 		 * Force a typecast here so that we don't emit something like "GROUP

contrib/postgres_fdw/expected/postgres_fdw.out

@@ -4590,45 +4590,35 @@ static bool
 foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
 {
 	Query	   *query = root->parse;
-	PathTarget *grouping_target;
+	PathTarget *grouping_target = root->upper_targets[UPPERREL_GROUP_AGG];
 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) grouped_rel->fdw_private;
 	PgFdwRelationInfo *ofpinfo;
 	List	   *aggvars;
 	ListCell   *lc;
 	int			i;
 	List	   *tlist = NIL;
 
-	/* Grouping Sets are not pushable */
+	/* We currently don't support pushing Grouping Sets. */
 	if (query->groupingSets)
 		return false;
 
 	/* Get the fpinfo of the underlying scan relation. */
 	ofpinfo = (PgFdwRelationInfo *) fpinfo->outerrel->fdw_private;
 
 	/*
-	 * If underneath input relation has any local conditions, those conditions
+	 * If underlying scan relation has any local conditions, those conditions
 	 * are required to be applied before performing aggregation.  Hence the
 	 * aggregate cannot be pushed down.
 	 */
 	if (ofpinfo->local_conds)
 		return false;
 
 	/*
-	 * The targetlist expected from this node and the targetlist pushed down
-	 * to the foreign server may be different. The latter requires
-	 * sortgrouprefs to be set to push down GROUP BY clause, but should not
-	 * have those arising from ORDER BY clause. These sortgrouprefs may be
-	 * different from those in the plan's targetlist. Use a copy of path
-	 * target to record the new sortgrouprefs.
-	 */
-	grouping_target = copy_pathtarget(root->upper_targets[UPPERREL_GROUP_AGG]);
-
-	/*
-	 * Evaluate grouping targets and check whether they are safe to push down
-	 * to the foreign side.  All GROUP BY expressions will be part of the
-	 * grouping target and thus there is no need to evaluate it separately.
-	 * While doing so, add required expressions into target list which can
-	 * then be used to pass to foreign server.
+	 * Examine grouping expressions, as well as other expressions we'd need to
+	 * compute, and check whether they are safe to push down to the foreign
+	 * server.  All GROUP BY expressions will be part of the grouping target
+	 * and thus there is no need to search for them separately.  Add grouping
+	 * expressions into target list which will be passed to foreign server.
 	 */
 	i = 0;
 	foreach(lc, grouping_target->exprs)
@@ -4640,51 +4630,59 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
 		/* Check whether this expression is part of GROUP BY clause */
 		if (sgref && get_sortgroupref_clause_noerr(sgref, query->groupClause))
 		{
+			TargetEntry *tle;
+
 			/*
-			 * If any of the GROUP BY expression is not shippable we can not
+			 * If any GROUP BY expression is not shippable, then we cannot
 			 * push down aggregation to the foreign server.
 			 */
 			if (!is_foreign_expr(root, grouped_rel, expr))
 				return false;
 
-			/* Pushable, add to tlist */
-			tlist = add_to_flat_tlist(tlist, list_make1(expr));
+			/*
+			 * Pushable, so add to tlist.  We need to create a TLE for this
+			 * expression and apply the sortgroupref to it.  We cannot use
+			 * add_to_flat_tlist() here because that avoids making duplicate
+			 * entries in the tlist.  If there are duplicate entries with
+			 * distinct sortgrouprefs, we have to duplicate that situation in
+			 * the output tlist.
+			 */
+			tle = makeTargetEntry(expr, list_length(tlist) + 1, NULL, false);
+			tle->ressortgroupref = sgref;
+			tlist = lappend(tlist, tle);
 		}
 		else
 		{
-			/* Check entire expression whether it is pushable or not */
+			/*
+			 * Non-grouping expression we need to compute.  Is it shippable?
+			 */
 			if (is_foreign_expr(root, grouped_rel, expr))
 			{
-				/* Pushable, add to tlist */
+				/* Yes, so add to tlist as-is; OK to suppress duplicates */
 				tlist = add_to_flat_tlist(tlist, list_make1(expr));
 			}
 			else
 			{
-				/*
-				 * If we have sortgroupref set, then it means that we have an
-				 * ORDER BY entry pointing to this expression.  Since we are
-				 * not pushing ORDER BY with GROUP BY, clear it.
-				 */
-				if (sgref)
-					grouping_target->sortgrouprefs[i] = 0;
-
-				/* Not matched exactly, pull the var with aggregates then */
+				/* Not pushable as a whole; extract its Vars and aggregates */
 				aggvars = pull_var_clause((Node *) expr,
 										  PVC_INCLUDE_AGGREGATES);
 
+				/*
+				 * If any aggregate expression is not shippable, then we
+				 * cannot push down aggregation to the foreign server.
+				 */
 				if (!is_foreign_expr(root, grouped_rel, (Expr *) aggvars))
 					return false;
 
 				/*
-				 * Add aggregates, if any, into the targetlist.  Plain var
-				 * nodes should be either same as some GROUP BY expression or
-				 * part of some GROUP BY expression. In later case, the query
-				 * cannot refer plain var nodes without the surrounding
-				 * expression.  In both the cases, they are already part of
+				 * Add aggregates, if any, into the targetlist.  Plain Vars
+				 * outside an aggregate can be ignored, because they should be
+				 * either same as some GROUP BY column or part of some GROUP
+				 * BY expression.  In either case, they are already part of
 				 * the targetlist and thus no need to add them again.  In fact
-				 * adding pulled plain var nodes in SELECT clause will cause
-				 * an error on the foreign server if they are not same as some
-				 * GROUP BY expression.
+				 * including plain Vars in the tlist when they do not match a
+				 * GROUP BY column would cause the foreign server to complain
+				 * that the shipped query is invalid.
 				 */
 				foreach(l, aggvars)
 				{
@@ -4700,7 +4698,7 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
 	}
 
 	/*
-	 * Classify the pushable and non-pushable having clauses and save them in
+	 * Classify the pushable and non-pushable HAVING clauses and save them in
 	 * remote_conds and local_conds of the grouped rel's fpinfo.
 	 */
 	if (root->hasHavingQual && query->havingQual)
@@ -4770,9 +4768,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
 		}
 	}
 
-	/* Transfer any sortgroupref data to the replacement tlist */
-	apply_pathtarget_labeling_to_tlist(tlist, grouping_target);
-
 	/* Store generated targetlist */
 	fpinfo->grouped_tlist = tlist;
 

contrib/postgres_fdw/postgres_fdw.c

@@ -636,6 +636,12 @@ explain (verbose, costs off)
 select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
 select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
 
+-- GROUP BY clause referring to same column multiple times
+-- Also, ORDER BY contains an aggregate function
+explain (verbose, costs off)
+select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
+select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
+
 -- Testing HAVING clause shippability
 explain (verbose, costs off)
 select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;

contrib/postgres_fdw/sql/postgres_fdw.sql

@@ -1003,11 +1003,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 
 	/*
 	 * The subquery's targetlist items are now in the appropriate form to
-	 * insert into the top query, but if we are under an outer join then
-	 * non-nullable items and lateral references may have to be turned into
-	 * PlaceHolderVars.  If we are dealing with an appendrel member then
-	 * anything that's not a simple Var has to be turned into a
-	 * PlaceHolderVar.  Set up required context data for pullup_replace_vars.
+	 * insert into the top query, except that we may need to wrap them in
+	 * PlaceHolderVars.  Set up required context data for pullup_replace_vars.
 	 */
 	rvcontext.root = root;
 	rvcontext.targetlist = subquery->targetList;
@@ -1019,13 +1016,48 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 		rvcontext.relids = NULL;
 	rvcontext.outer_hasSubLinks = &parse->hasSubLinks;
 	rvcontext.varno = varno;
-	rvcontext.need_phvs = (lowest_nulling_outer_join != NULL ||
-						   containing_appendrel != NULL);
-	rvcontext.wrap_non_vars = (containing_appendrel != NULL);
+	/* these flags will be set below, if needed */
+	rvcontext.need_phvs = false;
+	rvcontext.wrap_non_vars = false;
 	/* initialize cache array with indexes 0 .. length(tlist) */
 	rvcontext.rv_cache = palloc0((list_length(subquery->targetList) + 1) *
 								 sizeof(Node *));
 
+	/*
+	 * If we are under an outer join then non-nullable items and lateral
+	 * references may have to be turned into PlaceHolderVars.
+	 */
+	if (lowest_nulling_outer_join != NULL)
+		rvcontext.need_phvs = true;
+
+	/*
+	 * If we are dealing with an appendrel member then anything that's not a
+	 * simple Var has to be turned into a PlaceHolderVar.  We force this to
+	 * ensure that what we pull up doesn't get merged into a surrounding
+	 * expression during later processing and then fail to match the
+	 * expression actually available from the appendrel.
+	 */
+	if (containing_appendrel != NULL)
+	{
+		rvcontext.need_phvs = true;
+		rvcontext.wrap_non_vars = true;
+	}
+
+	/*
+	 * If the parent query uses grouping sets, we need a PlaceHolderVar for
+	 * anything that's not a simple Var.  Again, this ensures that expressions
+	 * retain their separate identity so that they will match grouping set
+	 * columns when appropriate.  (It'd be sufficient to wrap values used in
+	 * grouping set columns, and do so only in non-aggregated portions of the
+	 * tlist and havingQual, but that would require a lot of infrastructure
+	 * that pullup_replace_vars hasn't currently got.)
+	 */
+	if (parse->groupingSets)
+	{
+		rvcontext.need_phvs = true;
+		rvcontext.wrap_non_vars = true;
+	}
+
 	/*
 	 * Replace all of the top query's references to the subquery's outputs
 	 * with copies of the adjusted subtlist items, being careful not to

src/backend/optimizer/prep/prepjointree.c

@@ -1681,15 +1681,15 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
 		 */
 		if (old_attno < newnatts &&
 			(att = new_tupdesc->attrs[old_attno]) != NULL &&
-			!att->attisdropped && att->attinhcount != 0 &&
+			!att->attisdropped &&
 			strcmp(attname, NameStr(att->attname)) == 0)
 			new_attno = old_attno;
 		else
 		{
 			for (new_attno = 0; new_attno < newnatts; new_attno++)
 			{
 				att = new_tupdesc->attrs[new_attno];
-				if (!att->attisdropped && att->attinhcount != 0 &&
+				if (!att->attisdropped &&
 					strcmp(attname, NameStr(att->attname)) == 0)
 					break;
 			}

src/backend/optimizer/prep/prepunion.c

@@ -609,7 +609,17 @@ ECPGdump_a_struct(FILE *o, const char *name, const char *ind_name, char *arrsize
 						prefix, ind_prefix, arrsize, type->struct_sizeof,
 						(ind_p != NULL) ? ind_type->struct_sizeof : NULL);
 		if (ind_p != NULL && ind_p != &struct_no_indicator)
+		{
 			ind_p = ind_p->next;
+			if (ind_p == NULL && p->next != NULL) {
+				mmerror(PARSE_ERROR, ET_WARNING, "indicator struct \"%s\" has too few members", ind_name);
+				ind_p = &struct_no_indicator;
+			}
+		}
+	}
+
+	if (ind_type != NULL && ind_p != NULL && ind_p != &struct_no_indicator) {
+		mmerror(PARSE_ERROR, ET_WARNING, "indicator struct \"%s\" has too many members", ind_name);
 	}
 
 	free(pbuf);

src/interfaces/ecpg/preproc/type.c

@@ -0,0 +1,57 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1b s1delc1 s2sel s1c s2sel
+step s1b: BEGIN;
+step s1delc1: ALTER TABLE c1 NO INHERIT p;
+step s2sel: SELECT SUM(a) FROM p; <waiting ...>
+step s1c: COMMIT;
+step s2sel: <... completed>
+sum            
+
+11             
+step s2sel: SELECT SUM(a) FROM p;
+sum            
+
+1              
+
+starting permutation: s1b s1delc1 s1addc2 s2sel s1c s2sel
+step s1b: BEGIN;
+step s1delc1: ALTER TABLE c1 NO INHERIT p;
+step s1addc2: ALTER TABLE c2 INHERIT p;
+step s2sel: SELECT SUM(a) FROM p; <waiting ...>
+step s1c: COMMIT;
+step s2sel: <... completed>
+sum            
+
+11             
+step s2sel: SELECT SUM(a) FROM p;
+sum            
+
+101            
+
+starting permutation: s1b s1dropc1 s2sel s1c s2sel
+step s1b: BEGIN;
+step s1dropc1: DROP TABLE c1;
+step s2sel: SELECT SUM(a) FROM p; <waiting ...>
+step s1c: COMMIT;
+step s2sel: <... completed>
+sum            
+
+1              
+step s2sel: SELECT SUM(a) FROM p;
+sum            
+
+1              
+
+starting permutation: s1b s1delc1 s1modc1a s2sel s1c s2sel
+step s1b: BEGIN;
+step s1delc1: ALTER TABLE c1 NO INHERIT p;
+step s1modc1a: ALTER TABLE c1 ALTER COLUMN a TYPE float;
+step s2sel: SELECT SUM(a) FROM p; <waiting ...>
+step s1c: COMMIT;
+step s2sel: <... completed>
+error in steps s1c s2sel: ERROR:  attribute "a" of relation "c1" does not match parent's type
+step s2sel: SELECT SUM(a) FROM p;
+sum            
+
+1              

src/test/isolation/expected/alter-table-4.out

@@ -59,6 +59,7 @@ test: multiple-cic
 test: alter-table-1
 test: alter-table-2
 test: alter-table-3
+test: alter-table-4
 test: create-trigger
 test: sequence-ddl
 test: async-notify