[tarantool-patches] [PATCH v4 2/4] sql: replace flag MINMAX with flags MIN and MAX

[Kirill Shcherbatov]

Replaced sql function flag SQL_FUNC_MINMAX with couple new flags
SQL_FUNC_MIN and SQL_FUNC_MAX. This allows to distinguish MIN and
MAX function by flag instead of using user_data context.

This allows to delete user_data field in FundDef object in
further refactoring.

Needed for #2200, #4113, #2233
---
 src/box/sql/sqlInt.h  | 12 +++++++-----
 src/box/sql/func.c    | 31 +++++++++++++++----------------
 src/box/sql/resolve.c | 16 ++++++----------
 3 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 5a3e8f1c1..d71fd06d7 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1296,7 +1296,7 @@ struct FuncDestructor {
  * are assert() statements in the code to verify this.
  *
  * Value constraints (enforced via assert()):
- *     SQL_FUNC_MINMAX    ==  NC_MinMaxAgg      == SF_MinMaxAgg
+ *     NC_MinMaxAgg      == SF_MinMaxAgg
  *     SQL_FUNC_LENGTH    ==  OPFLAG_LENGTHARG
  *     SQL_FUNC_TYPEOF    ==  OPFLAG_TYPEOFARG
  *     SQL_FUNC_CONSTANT  ==  sql_DETERMINISTIC from the API
@@ -1317,8 +1317,10 @@ struct FuncDestructor {
 #define SQL_FUNC_COALESCE 0x0200	/* Built-in coalesce() or ifnull() */
 #define SQL_FUNC_UNLIKELY 0x0400	/* Built-in unlikely() function */
 #define SQL_FUNC_CONSTANT 0x0800	/* Constant inputs give a constant output */
-#define SQL_FUNC_MINMAX   0x1000	/* True for min() and max() aggregates */
-
+/** Built-in min() or least() function. */
+#define SQL_FUNC_MIN      0x1000
+/** Built-in max() or greatest() function. */
+#define SQL_FUNC_MAX      0x2000
 /**
  * If function returns string, it may require collation to be
  * applied on its result. For instance, result of substr()
@@ -2014,7 +2016,7 @@ struct NameContext {
  *
  * Value constraints (all checked via assert()):
  *    NC_HasAgg    == SF_HasAgg
- *    NC_MinMaxAgg == SF_MinMaxAgg == sql_FUNC_MINMAX
+ *    NC_MinMaxAgg == SF_MinMaxAgg
  *
  */
 #define NC_AllowAgg  0x0001	/* Aggregate functions are allowed here */
@@ -2072,7 +2074,7 @@ struct Select {
  *
  * Value constraints (all checked via assert())
  *     SF_HasAgg     == NC_HasAgg
- *     SF_MinMaxAgg  == NC_MinMaxAgg     == sql_FUNC_MINMAX
+ *     SF_MinMaxAgg  == NC_MinMaxAgg
  *     SF_FixedLimit == WHERE_USE_LIMIT
  */
 #define SF_Distinct       0x00001	/* Output should be DISTINCT */
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 07c019db9..a46df60ed 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -77,12 +77,12 @@ static void
 minmaxFunc(sql_context * context, int argc, sql_value ** argv)
 {
 	int i;
-	int mask;		/* 0 for min() or 0xffffffff for max() */
 	int iBest;
 	struct coll *pColl;
+	struct FuncDef *func = context->pFunc;
 
 	assert(argc > 1);
-	mask = sql_user_data(context) == 0 ? 0 : -1;
+	int mask = (func->funcFlags & SQL_FUNC_MAX) != 0 ? -1 : 0;
 	pColl = sqlGetFuncCollSeq(context);
 	assert(mask == -1 || mask == 0);
 	iBest = 0;
@@ -1721,20 +1721,17 @@ minmaxStep(sql_context * context, int NotUsed, sql_value ** argv)
 		if (pBest->flags)
 			sqlSkipAccumulatorLoad(context);
 	} else if (pBest->flags) {
-		int max;
 		int cmp;
 		struct coll *pColl = sqlGetFuncCollSeq(context);
-		/* This step function is used for both the min() and max() aggregates,
-		 * the only difference between the two being that the sense of the
-		 * comparison is inverted. For the max() aggregate, the
-		 * sql_user_data() function returns (void *)-1. For min() it
-		 * returns (void *)db, where db is the sql* database pointer.
-		 * Therefore the next statement sets variable 'max' to 1 for the max()
-		 * aggregate, or 0 for min().
+		/*
+		 * This step function is used for both the min()
+		 * and max() aggregates, the only difference
+		 * between the two being that the sense of the
+		 * comparison is inverted.
 		 */
-		max = sql_user_data(context) != 0;
+		bool is_max = (context->pFunc->funcFlags & SQL_FUNC_MAX) != 0;
 		cmp = sqlMemCompare(pBest, pArg, pColl);
-		if ((max && cmp < 0) || (!max && cmp > 0)) {
+		if ((is_max && cmp < 0) || (!is_max && cmp > 0)) {
 			sqlVdbeMemCopy(pBest, pArg);
 		} else {
 			sqlSkipAccumulatorLoad(context);
@@ -1861,12 +1858,14 @@ sqlRegisterBuiltinFunctions(void)
 		FUNCTION_COLL(trim, 1, 3, 0, trim_func),
 		FUNCTION_COLL(trim, 2, 3, 0, trim_func),
 		FUNCTION_COLL(trim, 3, 3, 0, trim_func),
-		FUNCTION(least, -1, 0, 1, minmaxFunc, FIELD_TYPE_SCALAR),
+		FUNCTION2(least, -1, 0, 1, minmaxFunc, SQL_FUNC_MIN,
+			  FIELD_TYPE_SCALAR),
 		AGGREGATE2(min, 1, 0, 1, minmaxStep, minMaxFinalize,
-			   SQL_FUNC_MINMAX, FIELD_TYPE_SCALAR),
-		FUNCTION(greatest, -1, 1, 1, minmaxFunc, FIELD_TYPE_SCALAR),
+			   SQL_FUNC_MIN, FIELD_TYPE_SCALAR),
+		FUNCTION2(greatest, -1, 1, 1, minmaxFunc, SQL_FUNC_MAX,
+			  FIELD_TYPE_SCALAR),
 		AGGREGATE2(max, 1, 1, 1, minmaxStep, minMaxFinalize,
-			   SQL_FUNC_MINMAX, FIELD_TYPE_SCALAR),
+			   SQL_FUNC_MAX, FIELD_TYPE_SCALAR),
 		FUNCTION2(typeof, 1, 0, 0, typeofFunc, SQL_FUNC_TYPEOF,
 			  FIELD_TYPE_STRING),
 		FUNCTION2(length, 1, 0, 0, lengthFunc, SQL_FUNC_LENGTH,
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 207f57ba6..83dff47b6 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -700,16 +700,12 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 				}
 				assert(pDef != 0);
 				if (pNC2) {
-					assert(SQL_FUNC_MINMAX ==
-					       NC_MinMaxAgg);
-					testcase((pDef->
-						  funcFlags &
-						  SQL_FUNC_MINMAX) != 0);
-					pNC2->ncFlags |=
-					    NC_HasAgg | (pDef->
-							 funcFlags &
-							 SQL_FUNC_MINMAX);
-
+					pNC2->ncFlags |= NC_HasAgg;
+					if ((pDef->funcFlags &
+					    SQL_FUNC_MIN) != 0 ||
+					    (pDef->funcFlags &
+					    SQL_FUNC_MAX) != 0)
+						pNC2->ncFlags |= NC_MinMaxAgg;
 				}
 				pNC->ncFlags |= NC_AllowAgg;
 			}
-- 
2.22.1