First cut of getting rid of GroupedVar.int/grouping_sets

This very clearly needs some more work, but to me the resulting
simplification seems to be pretty obviously worth it.

10 files changed, 46 insertions, 279 deletions

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index 97bfbbc326..782147f69c 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -74,8 +74,6 @@ static Datum ExecEvalScalarVar(ExprState *exprstate, ExprContext *econtext,
 				  bool *isNull, ExprDoneCond *isDone);
 static Datum ExecEvalScalarVarFast(ExprState *exprstate, ExprContext *econtext,
 					  bool *isNull, ExprDoneCond *isDone);
-static Datum ExecEvalScalarGroupedVarFast(ExprState *exprstate, ExprContext *econtext,
-					  bool *isNull, ExprDoneCond *isDone);
 static Datum ExecEvalWholeRowVar(WholeRowVarExprState *wrvstate,
 					ExprContext *econtext,
 					bool *isNull, ExprDoneCond *isDone);
@@ -563,8 +561,6 @@ ExecEvalWindowFunc(WindowFuncExprState *wfunc, ExprContext *econtext,
  * Note: ExecEvalScalarVar is executed only the first time through in a given
  * plan; it changes the ExprState's function pointer to pass control directly
  * to ExecEvalScalarVarFast after making one-time checks.
- *
- * We share this code with GroupedVar for simplicity.
  * ----------------------------------------------------------------
  */
 static Datum
@@ -642,24 +638,8 @@ ExecEvalScalarVar(ExprState *exprstate, ExprContext *econtext,
 		}
 	}
 
-	if (IsA(variable, GroupedVar))
-	{
-		Assert(variable->varno == OUTER_VAR);
-
-		/* Skip the checking on future executions of node */
-		exprstate->evalfunc = ExecEvalScalarGroupedVarFast;
-
-		if (!bms_is_member(attnum, econtext->grouped_cols))
-		{
-			*isNull = true;
-			return (Datum) 0;
-		}
-	}
-	else
-	{
-		/* Skip the checking on future executions of node */
-		exprstate->evalfunc = ExecEvalScalarVarFast;
-	}
+	/* Skip the checking on future executions of node */
+	exprstate->evalfunc = ExecEvalScalarVarFast;
 
 	/* Fetch the value from the slot */
 	return slot_getattr(slot, attnum, isNull);
@@ -707,31 +687,6 @@ ExecEvalScalarVarFast(ExprState *exprstate, ExprContext *econtext,
 	return slot_getattr(slot, attnum, isNull);
 }
 
-static Datum
-ExecEvalScalarGroupedVarFast(ExprState *exprstate, ExprContext *econtext,
-							 bool *isNull, ExprDoneCond *isDone)
-{
-	GroupedVar *variable = (GroupedVar *) exprstate->expr;
-	TupleTableSlot *slot;
-	AttrNumber	attnum;
-
-	if (isDone)
-		*isDone = ExprSingleResult;
-
-	slot = econtext->ecxt_outertuple;
-
-	attnum = variable->varattno;
-
-	if (!bms_is_member(attnum, econtext->grouped_cols))
-	{
-		*isNull = true;
-		return (Datum) 0;
-	}
-
-	/* Fetch the value from the slot */
-	return slot_getattr(slot, attnum, isNull);
-}
-
 /* ----------------------------------------------------------------
  *		ExecEvalWholeRowVar
  *
@@ -4504,11 +4459,6 @@ ExecInitExpr(Expr *node, PlanState *parent)
 				state->evalfunc = ExecEvalScalarVar;
 			}
 			break;
-		case T_GroupedVar:
-			Assert(((Var *) node)->varattno != InvalidAttrNumber);
-			state = (ExprState *) makeNode(ExprState);
-			state->evalfunc = ExecEvalScalarVar;
-			break;
 		case T_Const:
 			state = (ExprState *) makeNode(ExprState);
 			state->evalfunc = ExecEvalConst;
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 9b93a0ec85..65f9db8354 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -1028,9 +1028,41 @@ finalize_aggregates(AggState *aggstate,
 
 	aggstate->current_set = currentSet;
 
+	/*
+	 * Set all columns that are not in the current grouping set to NULL. It'd
+	 * arguably be cleaner to do this
+	 */
 	if (aggstate->grouped_cols)
+	{
+		TupleTableSlot *slot = econtext->ecxt_outertuple;
+		int attno;
+		TupleDesc tupdesc = slot->tts_tupleDescriptor;
+
 		econtext->grouped_cols = aggstate->grouped_cols[currentSet];
 
+		if (slot->tts_isempty)
+		{
+			/*
+			 * Force all values to be NULL if working on an empty input tuple
+			 * (i.e. an empty grouping set).  Perhaps better do that in
+			 * agg_retrieve_direct()?
+			 */
+			ExecStoreAllNullTuple(slot);
+		}
+		else
+		{
+			/* Force the tuple to be "fully virtual", in a pretty hacky way */
+			slot_getallattrs(slot);
+
+			for (attno = 0; attno < tupdesc->natts; attno++)
+			{
+				int attnum = tupdesc->attrs[attno]->attnum;
+				if (!bms_is_member(attnum, econtext->grouped_cols))
+					slot->tts_isnull[attno] = true;
+			}
+		}
+	}
+
 	for (aggno = 0; aggno < aggstate->numaggs; aggno++)
 	{
 		AggStatePerAgg peraggstate = &peragg[aggno];
@@ -1540,6 +1572,9 @@ agg_retrieve_direct(AggState *aggstate)
 						 * Note that this implies that there can't be any
 						 * references to ungrouped Vars, which would otherwise
 						 * cause issues with the empty output slot.
+						 *
+						 * XXX: This is no longer true, we currently deal with
+						 * this in finalize_aggregates().
 						 */
 						aggstate->input_done = true;
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index bcf6f791fe..214726e8e9 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1113,27 +1113,6 @@ _copyVar(const Var *from)
 }
 
 /*
- * _copyGroupedVar
- */
-static GroupedVar *
-_copyGroupedVar(const GroupedVar *from)
-{
-	GroupedVar		   *newnode = makeNode(GroupedVar);
-
-	COPY_SCALAR_FIELD(varno);
-	COPY_SCALAR_FIELD(varattno);
-	COPY_SCALAR_FIELD(vartype);
-	COPY_SCALAR_FIELD(vartypmod);
-	COPY_SCALAR_FIELD(varcollid);
-	COPY_SCALAR_FIELD(varlevelsup);
-	COPY_SCALAR_FIELD(varnoold);
-	COPY_SCALAR_FIELD(varoattno);
-	COPY_LOCATION_FIELD(location);
-
-	return newnode;
-}
-
-/*
  * _copyConst
  */
 static Const *
@@ -4297,9 +4276,6 @@ copyObject(const void *from)
 		case T_Var:
 			retval = _copyVar(from);
 			break;
-		case T_GroupedVar:
-			retval = _copyGroupedVar(from);
-			break;
 		case T_Const:
 			retval = _copyConst(from);
 			break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index d3868ab449..38cacf1a36 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -153,22 +153,6 @@ _equalVar(const Var *a, const Var *b)
 }
 
 static bool
-_equalGroupedVar(const GroupedVar *a, const GroupedVar *b)
-{
-	COMPARE_SCALAR_FIELD(varno);
-	COMPARE_SCALAR_FIELD(varattno);
-	COMPARE_SCALAR_FIELD(vartype);
-	COMPARE_SCALAR_FIELD(vartypmod);
-	COMPARE_SCALAR_FIELD(varcollid);
-	COMPARE_SCALAR_FIELD(varlevelsup);
-	COMPARE_SCALAR_FIELD(varnoold);
-	COMPARE_SCALAR_FIELD(varoattno);
-	COMPARE_LOCATION_FIELD(location);
-
-	return true;
-}
-
-static bool
 _equalConst(const Const *a, const Const *b)
 {
 	COMPARE_SCALAR_FIELD(consttype);
@@ -2693,9 +2677,6 @@ equal(const void *a, const void *b)
 		case T_Var:
 			retval = _equalVar(a, b);
 			break;
-		case T_GroupedVar:
-			retval = _equalGroupedVar(a, b);
-			break;
 		case T_Const:
 			retval = _equalConst(a, b);
 			break;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 20f2ebb3b0..282a46b76f 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -45,9 +45,6 @@ exprType(const Node *expr)
 		case T_Var:
 			type = ((const Var *) expr)->vartype;
 			break;
-		case T_GroupedVar:
-			type = ((const GroupedVar *) expr)->vartype;
-			break;
 		case T_Const:
 			type = ((const Const *) expr)->consttype;
 			break;
@@ -274,8 +271,6 @@ exprTypmod(const Node *expr)
 	{
 		case T_Var:
 			return ((const Var *) expr)->vartypmod;
-		case T_GroupedVar:
-			return ((const GroupedVar *) expr)->vartypmod;
 		case T_Const:
 			return ((const Const *) expr)->consttypmod;
 		case T_Param:
@@ -749,9 +744,6 @@ exprCollation(const Node *expr)
 		case T_Var:
 			coll = ((const Var *) expr)->varcollid;
 			break;
-		case T_GroupedVar:
-			coll = ((const GroupedVar *) expr)->varcollid;
-			break;
 		case T_Const:
 			coll = ((const Const *) expr)->constcollid;
 			break;
@@ -991,9 +983,6 @@ exprSetCollation(Node *expr, Oid collation)
 		case T_Var:
 			((Var *) expr)->varcollid = collation;
 			break;
-		case T_GroupedVar:
-			((GroupedVar *) expr)->varcollid = collation;
-			break;
 		case T_Const:
 			((Const *) expr)->constcollid = collation;
 			break;
@@ -1212,9 +1201,6 @@ exprLocation(const Node *expr)
 		case T_Var:
 			loc = ((const Var *) expr)->location;
 			break;
-		case T_GroupedVar:
-			loc = ((const GroupedVar *) expr)->location;
-			break;
 		case T_Const:
 			loc = ((const Const *) expr)->location;
 			break;
@@ -1681,7 +1667,6 @@ expression_tree_walker(Node *node,
 	switch (nodeTag(node))
 	{
 		case T_Var:
-		case T_GroupedVar:
 		case T_Const:
 		case T_Param:
 		case T_CoerceToDomainValue:
@@ -2231,15 +2216,6 @@ expression_tree_mutator(Node *node,
 				return (Node *) newnode;
 			}
 			break;
-		case T_GroupedVar:
-			{
-				GroupedVar         *groupedvar = (GroupedVar *) node;
-				GroupedVar		   *newnode;
-
-				FLATCOPY(newnode, groupedvar, GroupedVar);
-				return (Node *) newnode;
-			}
-			break;
 		case T_Const:
 			{
 				Const	   *oldnode = (Const *) node;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e985ed68f9..65da8e8d20 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -945,22 +945,6 @@ _outVar(StringInfo str, const Var *node)
 }
 
 static void
-_outGroupedVar(StringInfo str, const GroupedVar *node)
-{
-	WRITE_NODE_TYPE("GROUPEDVAR");
-
-	WRITE_UINT_FIELD(varno);
-	WRITE_INT_FIELD(varattno);
-	WRITE_OID_FIELD(vartype);
-	WRITE_INT_FIELD(vartypmod);
-	WRITE_OID_FIELD(varcollid);
-	WRITE_UINT_FIELD(varlevelsup);
-	WRITE_UINT_FIELD(varnoold);
-	WRITE_INT_FIELD(varoattno);
-	WRITE_LOCATION_FIELD(location);
-}
-
-static void
 _outConst(StringInfo str, const Const *node)
 {
 	WRITE_NODE_TYPE("CONST");
@@ -3078,9 +3062,6 @@ _outNode(StringInfo str, const void *obj)
 			case T_Var:
 				_outVar(str, obj);
 				break;
-			case T_GroupedVar:
-				_outGroupedVar(str, obj);
-				break;
 			case T_Const:
 				_outConst(str, obj);
 				break;
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 09707c7eed..9d58414e71 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -459,27 +459,6 @@ _readVar(void)
 }
 
 /*
- * _readGroupedVar
- */
-static GroupedVar *
-_readGroupedVar(void)
-{
-	READ_LOCALS(GroupedVar);
-
-	READ_UINT_FIELD(varno);
-	READ_INT_FIELD(varattno);
-	READ_OID_FIELD(vartype);
-	READ_INT_FIELD(vartypmod);
-	READ_OID_FIELD(varcollid);
-	READ_UINT_FIELD(varlevelsup);
-	READ_UINT_FIELD(varnoold);
-	READ_INT_FIELD(varoattno);
-	READ_LOCATION_FIELD(location);
-
-	READ_DONE();
-}
-
-/*
  * _readConst
  */
 static Const *
@@ -1417,8 +1396,6 @@ parseNodeString(void)
 		return_value = _readIntoClause();
 	else if (MATCH("VAR", 3))
 		return_value = _readVar();
-	else if (MATCH("GROUPEDVAR", 10))
-		return_value = _readGroupedVar();
 	else if (MATCH("CONST", 5))
 		return_value = _readConst();
 	else if (MATCH("PARAM", 5))
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index f1fa1510c3..d4ac9ed25b 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -67,12 +67,6 @@ typedef struct
 	int			rtoffset;
 } fix_upper_expr_context;
 
-typedef struct
-{
-	PlannerInfo *root;
-	Bitmapset   *groupedcols;
-} set_group_vars_context;
-
 /*
  * Check if a Const node is a regclass value.  We accept plain OID too,
  * since a regclass Const will get folded to that type if it's an argument
@@ -145,9 +139,6 @@ static List *set_returning_clause_references(PlannerInfo *root,
 static bool fix_opfuncids_walker(Node *node, void *context);
 static bool extract_query_dependencies_walker(Node *node,
 								  PlannerInfo *context);
-static void set_group_vars(PlannerInfo *root, Agg *agg);
-static Node *set_group_vars_mutator(Node *node, set_group_vars_context *context);
-
 
 /*****************************************************************************
  *
@@ -661,7 +652,6 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 			else
 			{
 				set_upper_references(root, plan, rtoffset);
-				set_group_vars(root, (Agg *) plan);
 			}
 			break;
 		case T_Group:
@@ -1450,100 +1440,6 @@ fix_scan_expr_walker(Node *node, fix_scan_expr_context *context)
 								  (void *) context);
 }
 
-
-/*
- * set_group_vars
- *    Modify any Var references in the target list of a grouping sets
- *    containing Agg node to use GroupedVar instead, which will conditionally
- *    replace them with nulls at runtime.
- */
-static void
-set_group_vars(PlannerInfo *root, Agg *agg)
-{
-	set_group_vars_context context;
-	AttrNumber *groupColIdx = root->groupColIdx;
-	int			numCols = list_length(root->parse->groupClause);
-	int			i;
-	Bitmapset  *cols = NULL;
-
-	if (!agg->groupingSets)
-		return;
-
-	if (!groupColIdx)
-	{
-		Assert(numCols == agg->numCols);
-		groupColIdx = agg->grpColIdx;
-	}
-
-	context.root = root;
-
-	for (i = 0; i < numCols; ++i)
-		cols = bms_add_member(cols, groupColIdx[i]);
-
-	context.groupedcols = cols;
-
-	agg->plan.targetlist = (List *)
-		set_group_vars_mutator((Node *) agg->plan.targetlist,
-							   &context);
-	agg->plan.qual = (List *)
-		set_group_vars_mutator((Node *) agg->plan.qual,
-							   &context);
-}
-
-static Node *
-set_group_vars_mutator(Node *node, set_group_vars_context *context)
-{
-	if (node == NULL)
-		return NULL;
-	if (IsA(node, Var))
-	{
-		Var *var = (Var *) node;
-
-		if (var->varno == OUTER_VAR &&
-			bms_is_member(var->varattno, context->groupedcols))
-		{
-			var = copyVar(var);
-			var->xpr.type = T_GroupedVar;
-		}
-
-		return (Node *) var;
-	}
-	else if (IsA(node, Aggref))
-	{
-		/*
-		 * don't recurse into the arguments or filter of Aggrefs, since they
-		 * see the values prior to grouping.  But do recurse into direct args
-		 * if any.
-		 */
-
-		if (((Aggref *)node)->aggdirectargs != NIL)
-		{
-			Aggref *newnode = palloc(sizeof(Aggref));
-
-			memcpy(newnode, node, sizeof(Aggref));
-
-			newnode->aggdirectargs
-				= (List *) expression_tree_mutator((Node *) newnode->aggdirectargs,
-												   set_group_vars_mutator,
-												   (void *) context);
-
-			return (Node *) newnode;
-		}
-
-		return node;
-	}
-	else if (IsA(node, GroupingFunc))
-	{
-		/*
-		 * GroupingFuncs don't see the values at all.
-		 */
-		return node;
-	}
-	return expression_tree_mutator(node, set_group_vars_mutator,
-								   (void *) context);
-}
-
-
 /*
  * set_join_references
  *	  Modify the target list and quals of a join node to reference its
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 4e7b7a12c8..49b5ee08b1 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -4768,7 +4768,7 @@ get_target_list(List *targetList, deparse_context *context,
 		 * different from a whole-row Var).  We need to call get_variable
 		 * directly so that we can tell it to do the right thing.
 		 */
-		if (tle->expr && (IsA(tle->expr, Var) || IsA(tle->expr, GroupedVar)))
+		if (tle->expr && (IsA(tle->expr, Var)))
 		{
 			attname = get_variable((Var *) tle->expr, 0, true, context);
 		}
@@ -5808,10 +5808,10 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 		 * Force parentheses because our caller probably assumed a Var is a
 		 * simple expression.
 		 */
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, '(');
 		get_rule_expr((Node *) tle->expr, context, true);
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, ')');
 
 		pop_child_plan(dpns, &save_dpns);
@@ -5833,10 +5833,10 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 		 * Force parentheses because our caller probably assumed a Var is a
 		 * simple expression.
 		 */
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, '(');
 		get_rule_expr((Node *) tle->expr, context, true);
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, ')');
 
 		pop_child_plan(dpns, &save_dpns);
@@ -5856,10 +5856,10 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 		 * Force parentheses because our caller probably assumed a Var is a
 		 * simple expression.
 		 */
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, '(');
 		get_rule_expr((Node *) tle->expr, context, true);
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, ')');
 
 		return NULL;
@@ -5899,10 +5899,10 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 		 * Force parentheses because our caller probably assumed a Var is a
 		 * simple expression.
 		 */
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, '(');
 		get_rule_expr((Node *) tle->expr, context, true);
-		if (!IsA(tle->expr, Var) && !IsA(tle->expr, GroupedVar))
+		if (!IsA(tle->expr, Var))
 			appendStringInfoChar(buf, ')');
 
 		pop_child_plan(dpns, &save_dpns);
@@ -6933,10 +6933,6 @@ get_rule_expr(Node *node, deparse_context *context,
 			(void) get_variable((Var *) node, 0, false, context);
 			break;
 
-		case T_GroupedVar:
-			(void) get_variable((Var *) node, 0, false, context);
-			break;
-
 		case T_Const:
 			get_const_expr((Const *) node, context, 0);
 			break;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index e514d0d706..159b16fbf8 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -131,7 +131,6 @@ typedef enum NodeTag
 	T_RangeVar,
 	T_Expr,
 	T_Var,
-	T_GroupedVar,
 	T_Const,
 	T_Param,
 	T_Aggref,