Hi

čt 21. 2. 2019 v 22:05 odesílatel Tom Lane <t...@sss.pgh.pa.us> napsal:

> Pavel Stehule <pavel.steh...@gmail.com> writes:
> > čt 21. 2. 2019 v 3:20 odesílatel Chapman Flack <c...@anastigmatix.net>
> > napsal:
> >> I am not sure I have an answer to the objections being raised on grounds
> >> of taste. To me, it's persuasive that GREATEST and LEAST are described
> in
> >> the docco as functions, they are used much like variadic functions, and
> >> this patch allows them to be used in the ways you would expect variadic
> >> functions to be usable.
>
> > I wrote doc (just one sentence) and minimal test. Both can be enhanced.
>
> I remain of the opinion that this patch is a mess.
>
> I don't share Pavel's opinion that this is a clean user API, though
> I'll grant that others might have different opinions on that.
> I could hold my nose and overlook that if it led to a clean internal
> implementation.  But it doesn't: this patch just bolts a huge,
> undocumented wart onto the side of MinMaxExpr.  (The arguments are
> in the args field, except when they aren't?  And everyplace that
> deals with MinMaxExpr needs to know that, as well as the fact that
> the semantics are totally different?  Ick.)
>

fixed


> An example of the lack of care here is that the change in struct
> ExprEvalStep breaks that struct's size constraint:
>
>      * Inline data for the operation.  Inline data is faster to access, but
>      * also bloats the size of all instructions.  The union should be kept
> to
>      * no more than 40 bytes on 64-bit systems (so that the entire struct
> is
>      * no more than 64 bytes, a single cacheline on common systems).
>
>
fixed


> Andres is going to be quite displeased if that gets committed ;-).
>

I hope so I solved all your objections. Now, the patch is really reduced.



> I still say we should reject this and invent array_greatest/array_least
> functions instead.
>

I am not against these functions, but these functions doesn't solve a
confusing of some users, so LEAST, GREATEST looks like variadic functions,
but it doesn't allow VARIADIC parameter.

Comments, notes?



>                         regards, tom lane
>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 86ff4e5c9e..aa4e80cca7 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -12679,6 +12679,12 @@ SELECT NULLIF(value, '(none)') ...
 </synopsis>
 <synopsis>
 <function>LEAST</function>(<replaceable>value</replaceable> <optional>, ...</optional>)
+</synopsis>
+<synopsis>
+<function>GREATEST</function>(VARIADIC <replaceable>anyarray</replaceable>)
+</synopsis>
+<synopsis>
+<function>LEAST</function>(VARIADIC <replaceable>anyarray</replaceable>)
 </synopsis>
 
    <para>
@@ -12697,6 +12703,11 @@ SELECT NULLIF(value, '(none)') ...
     make them return NULL if any argument is NULL, rather than only when
     all are NULL.
    </para>
+
+   <para>
+    These functions supports passing parameters as a array after
+    <literal>VARIADIC</literal> keyword.
+   </para>
   </sect2>
  </sect1>
 
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index a018925d4e..418aca9c84 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -2819,6 +2819,55 @@ ExecEvalMinMax(ExprState *state, ExprEvalStep *op)
 	/* default to null result */
 	*op->resnull = true;
 
+	if (IsVariadicMinMax(operator))
+	{
+		ArrayIterator array_iterator;
+		ArrayType  *arr;
+		Datum	value;
+		bool	isnull;
+
+		if (nulls[0])
+			return;
+
+		arr = DatumGetArrayTypeP(values[0]);
+
+		array_iterator = array_create_iterator(arr, 0, NULL);
+		while (array_iterate(array_iterator, &value, &isnull))
+		{
+			if (isnull)
+				continue;
+
+			if (*op->resnull)
+			{
+				/* first nonnull input */
+				*op->resvalue = value;
+				*op->resnull = false;
+			}
+			else
+			{
+				int			cmpresult;
+
+				Assert(fcinfo->nargs == 2);
+
+				/* apply comparison function */
+				fcinfo->args[0].value = *op->resvalue;
+				fcinfo->args[1].value = value;
+
+				fcinfo->isnull = false;
+				cmpresult = DatumGetInt32(FunctionCallInvoke(fcinfo));
+				if (fcinfo->isnull) /* probably should not happen */
+					continue;
+
+				if (cmpresult > 0 && operator == IS_LEAST_VARIADIC)
+					*op->resvalue = value;
+				else if (cmpresult < 0 && operator == IS_GREATEST_VARIADIC)
+					*op->resvalue = value;
+			}
+		}
+
+		return;
+	}
+
 	for (off = 0; off < op->d.minmax.nelems; off++)
 	{
 		/* ignore NULL inputs */
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 8ed30c011a..efabaa0046 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -465,7 +465,8 @@ exprTypmod(const Node *expr)
 				int32		typmod;
 				ListCell   *arg;
 
-				if (exprType((Node *) linitial(mexpr->args)) != minmaxtype)
+				if (!IsVariadicMinMax(mexpr->op) &&
+						exprType((Node *) linitial(mexpr->args)) != minmaxtype)
 					return -1;
 				typmod = exprTypmod((Node *) linitial(mexpr->args));
 				if (typmod < 0)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a68f78e0e0..65f44a8df6 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -13803,6 +13803,22 @@ func_expr_common_subexpr:
 					v->location = @1;
 					$$ = (Node *)v;
 				}
+			| GREATEST '(' VARIADIC a_expr ')'
+				{
+					MinMaxExpr *v = makeNode(MinMaxExpr);
+					v->args = list_make1($4);
+					v->op = IS_GREATEST_VARIADIC;
+					v->location = @1;
+					$$ = (Node *)v;
+				}
+			| LEAST '(' VARIADIC a_expr ')'
+				{
+					MinMaxExpr *v = makeNode(MinMaxExpr);
+					v->args = list_make1($4);
+					v->op = IS_LEAST_VARIADIC;
+					v->location = @1;
+					$$ = (Node *)v;
+				}
 			| XMLCONCAT '(' expr_list ')'
 				{
 					$$ = makeXmlExpr(IS_XMLCONCAT, NULL, NIL, $3, @1);
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index e559353529..d739582ba0 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -2276,22 +2276,43 @@ transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m)
 		newargs = lappend(newargs, newe);
 	}
 
-	newm->minmaxtype = select_common_type(pstate, newargs, funcname, NULL);
-	/* minmaxcollid and inputcollid will be set by parse_collate.c */
+	if (IsVariadicMinMax(newm->op))
+	{
+		Oid		array_typid;
+		Oid		element_typid;
 
-	/* Convert arguments if necessary */
-	foreach(args, newargs)
+		array_typid = exprType(linitial(newargs));
+
+		if (array_typid == InvalidOid)
+			elog(ERROR, "cannot to determinate result type");
+
+		element_typid = get_base_element_type(array_typid);
+		if (!OidIsValid(element_typid))
+			elog(ERROR, "expected a array parameter");
+
+		newm->minmaxtype = element_typid;
+		newm->args = newargs;
+	}
+	else
 	{
-		Node	   *e = (Node *) lfirst(args);
-		Node	   *newe;
+		newm->minmaxtype = select_common_type(pstate, newargs, funcname, NULL);
+		/* minmaxcollid and inputcollid will be set by parse_collate.c */
 
-		newe = coerce_to_common_type(pstate, e,
-									 newm->minmaxtype,
-									 funcname);
-		newcoercedargs = lappend(newcoercedargs, newe);
+		/* Convert arguments if necessary */
+		foreach(args, newargs)
+		{
+			Node	   *e = (Node *) lfirst(args);
+			Node	   *newe;
+
+			newe = coerce_to_common_type(pstate, e,
+										 newm->minmaxtype,
+										 funcname);
+			newcoercedargs = lappend(newcoercedargs, newe);
+		}
+
+		newm->args = newcoercedargs;
 	}
 
-	newm->args = newcoercedargs;
 	newm->location = m->location;
 	return (Node *) newm;
 }
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 0e9598ebfe..013596756d 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1843,9 +1843,11 @@ FigureColnameInternal(Node *node, char **name)
 			switch (((MinMaxExpr *) node)->op)
 			{
 				case IS_GREATEST:
+				case IS_GREATEST_VARIADIC:
 					*name = "greatest";
 					return 2;
 				case IS_LEAST:
+				case IS_LEAST_VARIADIC:
 					*name = "least";
 					return 2;
 			}
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 1258092dc8..6e7ebb1861 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -8468,13 +8468,23 @@ get_rule_expr(Node *node, deparse_context *context,
 				switch (minmaxexpr->op)
 				{
 					case IS_GREATEST:
+					case IS_GREATEST_VARIADIC:
 						appendStringInfoString(buf, "GREATEST(");
 						break;
 					case IS_LEAST:
+					case IS_LEAST_VARIADIC:
 						appendStringInfoString(buf, "LEAST(");
 						break;
 				}
-				get_rule_expr((Node *) minmaxexpr->args, context, true);
+
+				if (IsVariadicMinMax(minmaxexpr->op))
+				{
+					appendStringInfoString(buf, "VARIADIC ");
+					get_rule_expr(linitial(minmaxexpr->args), context, true);
+				}
+				else
+					get_rule_expr((Node *) minmaxexpr->args, context, true);
+
 				appendStringInfoChar(buf, ')');
 			}
 			break;
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index a7efae7038..827fcf6d42 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1080,9 +1080,14 @@ typedef struct CoalesceExpr
 typedef enum MinMaxOp
 {
 	IS_GREATEST,
-	IS_LEAST
+	IS_LEAST,
+	IS_GREATEST_VARIADIC,
+	IS_LEAST_VARIADIC
 } MinMaxOp;
 
+#define IsVariadicMinMax(op)		(op == IS_GREATEST_VARIADIC || \
+									 op == IS_LEAST_VARIADIC)
+
 typedef struct MinMaxExpr
 {
 	Expr		xpr;
diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index c0c8acf035..eca127ed2d 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -392,3 +392,54 @@ ROLLBACK;
 --
 DROP TABLE CASE_TBL;
 DROP TABLE CASE2_TBL;
+--
+-- greatest and least tests
+--
+SELECT least(10,20,30,40);
+ least 
+-------
+    10
+(1 row)
+
+SELECT greatest(10,20,30,40);
+ greatest 
+----------
+       40
+(1 row)
+
+SELECT least(VARIADIC ARRAY[10,20,30,40]);
+ least 
+-------
+    10
+(1 row)
+
+SELECT greatest(VARIADIC ARRAY[10,20,30,40]);
+ greatest 
+----------
+       40
+(1 row)
+
+SELECT least(10,20,30,40, NULL);
+ least 
+-------
+    10
+(1 row)
+
+SELECT greatest(10,20,30,40, NULL);
+ greatest 
+----------
+       40
+(1 row)
+
+SELECT least(VARIADIC ARRAY[10,20,30,40, NULL]);
+ least 
+-------
+    10
+(1 row)
+
+SELECT greatest(VARIADIC ARRAY[10,20,30,40, NULL]);
+ greatest 
+----------
+       40
+(1 row)
+
diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql
index 17436c524a..c482676e17 100644
--- a/src/test/regress/sql/case.sql
+++ b/src/test/regress/sql/case.sql
@@ -252,3 +252,16 @@ ROLLBACK;
 
 DROP TABLE CASE_TBL;
 DROP TABLE CASE2_TBL;
+
+--
+-- greatest and least tests
+--
+SELECT least(10,20,30,40);
+SELECT greatest(10,20,30,40);
+SELECT least(VARIADIC ARRAY[10,20,30,40]);
+SELECT greatest(VARIADIC ARRAY[10,20,30,40]);
+
+SELECT least(10,20,30,40, NULL);
+SELECT greatest(10,20,30,40, NULL);
+SELECT least(VARIADIC ARRAY[10,20,30,40, NULL]);
+SELECT greatest(VARIADIC ARRAY[10,20,30,40, NULL]);

Reply via email to