[tarantool-patches] [PATCH 1/2] sql: add better collation determination in functions

[Ivan Koptelov]

Before the patch determination of collation in SQL functions
(used only in instr) was too narrow: the arguments were scanned
from left to right, till the argument with collation was
encountered, then its collation was used.
Now every arguments collation is considered. The right collation
which would be used in function is determined using ANSI
compatibility rules ("type combination" rules).

Note: currently only instr() a.k.a position() uses mechanism
described above, other functions (except aggregate) simply
ignores collations.
---
 src/box/sql/expr.c   | 69 ++++++++++++++++++++++++++++++++++++++++----
 src/box/sql/sqlInt.h |  8 ++++-
 2 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a2c70935e..2f48d90c6 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4093,16 +4093,73 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 					testcase(i == 31);
 					constMask |= MASKBIT32(i);
 				}
-				if ((pDef->funcFlags & SQL_FUNC_NEEDCOLL) !=
-				    0 && coll == NULL) {
-					bool unused;
-					uint32_t id;
+			}
+			/*
+			 * Function arguments may have different
+			 * collations. The following code
+			 * checks if they are compatible and
+			 * finds the collation to be used. This
+			 * is done using ANSI rules from
+			 * collations_check_compatibility().
+			 */
+			if ((pDef->funcFlags & SQL_FUNC_NEEDCOLL) != 0 &&
+			     coll == NULL) {
+				struct coll *unused = NULL;
+				uint32_t curr_id = COLL_NONE;
+				bool is_curr_forced = false;
+
+				uint32_t temp_id = COLL_NONE;
+				bool is_temp_forced = false;
+
+				uint32_t lhs_id = COLL_NONE;
+				bool is_lhs_forced = false;
+
+				uint32_t rhs_id = COLL_NONE;
+				bool is_rhs_forced = false;
+
+				for (int i = 0; i < nFarg; i++) {
 					if (sql_expr_coll(pParse,
 							  pFarg->a[i].pExpr,
-							  &unused, &id,
-							  &coll) != 0)
+							  &is_lhs_forced,
+							  &lhs_id,
+							  &unused) != 0)
 						return 0;
+
+					for (int j = i + 1; j < nFarg; j++) {
+						if (sql_expr_coll(pParse,
+								  pFarg->a[j].pExpr,
+								  &is_rhs_forced,
+								  &rhs_id,
+								  &unused) != 0)
+							return 0;
+
+						if (collations_check_compatibility(
+							lhs_id, is_lhs_forced,
+							rhs_id, is_rhs_forced,
+							&temp_id) != 0) {
+							pParse->rc =
+								SQL_TARANTOOL_ERROR;
+							pParse->nErr++;
+							return 0;
+						}
+
+						is_temp_forced = (temp_id ==
+								  lhs_id) ?
+								 is_lhs_forced :
+								 is_rhs_forced;
+
+						if (collations_check_compatibility(
+							curr_id, is_curr_forced,
+							temp_id, is_temp_forced,
+							&curr_id) != 0) {
+							pParse->rc =
+								SQL_TARANTOOL_ERROR;
+							pParse->nErr++;
+							return 0;
+						}
+					}
 				}
+				coll = coll_by_id(curr_id)->coll;
 			}
 			if (pFarg) {
 				if (constMask) {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 8967ea3e0..47ee474bb 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1660,7 +1660,13 @@ struct FuncDestructor {
 #define SQL_FUNC_LIKE     0x0004	/* Candidate for the LIKE optimization */
 #define SQL_FUNC_CASE     0x0008	/* Case-sensitive LIKE-type function */
 #define SQL_FUNC_EPHEM    0x0010	/* Ephemeral.  Delete with VDBE */
-#define SQL_FUNC_NEEDCOLL 0x0020	/* sqlGetFuncCollSeq() might be called */
+#define SQL_FUNC_NEEDCOLL 0x0020	/* sqlGetFuncCollSeq() might be called.
+					 * The flag is set when the collation
+					 * of function arguments should be
+					 * determined, using rules in
+					 * collations_check_compatibility()
+					 * function.
+					 */
 #define SQL_FUNC_LENGTH   0x0040	/* Built-in length() function */
 #define SQL_FUNC_TYPEOF   0x0080	/* Built-in typeof() function */
 #define SQL_FUNC_COUNT    0x0100	/* Built-in count(*) aggregate */
-- 
2.20.1