[tarantool-patches] [PATCH 3/3] sql: change collation compatibility rules

[Nikita Pettik <korablev@tarantool.org>]

Before this patch our SQL implementation relied on strange rules
when comparing strings with different collations:

- if either operand has an explicit collating function assignment using
  the postfix COLLATE operator, then the explicit collating function is
  used for comparison, with precedence to the collating function of the
  left operand;

- if either operand is a column, then the collating function of that
  column is used with precedence to the left operand.

The main concern in this implementation is that collation of the left
operand is forced over right one (even if both operands come with
explicit COLLATE clause). This contradicts ANSI SQL and seems to be
quite misleading, since if user simple swap LHS and RHS - result of
query may change.

Lets introduce restrictions concerning collations compatibilities.
Collations of LHS and RHS are compatible (i.e. no "Illegal mix of
collations" is thrown) if:

- one of collations is mentioned alongside with explicit COLLATE clause,
  which forces this collation over another one. It is allowed to have
  the same forced collations;

- both collations are derived from table columns and they are the same;

- one collation is derived from table column and another one is not
  specified (i.e. COLL_NONE).

The compound SELECT operators UNION, INTERSECT and EXCEPT perform implicit
comparisons between values. Hence, all new rules stated above are
applied to parts of compound SELECT. Otherwise, an error is raised.
In other words, before current patch queries like below were allowed:

SELECT 'abc' COLLATE binary UNION SELECT 'ABC' COLLATE "unicode_ci";
---
- - ['ABC']
  - ['abc']
...

If we swap collations, we will get another result:

SELECT 'ABC' COLLATE "unicode_ci" UNION SELECT 'abc' COLLATE BINARY
---
- - ['abc']
...

Now such queries are illegal.

Closes #3185
---
 src/box/errcode.h                 |   1 +
 src/box/sql/expr.c                |  70 ++++++++++++++++++------
 src/box/sql/select.c              | 111 +++++++++++++++++++++-----------------
 src/box/sql/sqliteInt.h           |  28 +++++++++-
 src/box/sql/where.c               |  13 +++--
 src/box/sql/whereexpr.c           |  13 ++---
 test/box/misc.result              |   1 +
 test/sql-tap/e_select1.test.lua   |  12 ++---
 test/sql-tap/in4.test.lua         |   2 +-
 test/sql-tap/join.test.lua        |   2 +-
 test/sql-tap/tkt3493.test.lua     |   4 +-
 test/sql-tap/transitive1.test.lua |   4 +-
 test/sql-tap/with1.test.lua       |   2 +-
 test/sql/collation.result         |  63 ++++++++++++++++++++++
 test/sql/collation.test.lua       |  22 ++++++++
 15 files changed, 252 insertions(+), 96 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 04f4f34ee..d16702978 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -223,6 +223,7 @@ struct errcode_record {
 	/*168 */_(ER_DROP_FK_CONSTRAINT,	"Failed to drop foreign key constraint '%s': %s") \
 	/*169 */_(ER_NO_SUCH_CONSTRAINT,	"Constraint %s does not exist") \
 	/*170 */_(ER_CONSTRAINT_EXISTS,		"Constraint %s already exists") \
+	/*171 */_(ER_ILLEGAL_COLLATION_MIX,	"Illegal mix of collations") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a13de4f0e..bd25ed656 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -161,10 +161,10 @@ sqlite3ExprSkipCollate(Expr * pExpr)
 }
 
 struct coll *
-sql_expr_coll(Parse *parse, Expr *p, bool *is_found, uint32_t *coll_id)
+sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id)
 {
 	struct coll *coll = NULL;
-	*is_found = false;
+	*is_explicit_coll = false;
 	*coll_id = COLL_NONE;
 	while (p != NULL) {
 		int op = p->op;
@@ -177,7 +177,7 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_found, uint32_t *coll_id)
 		if (op == TK_COLLATE ||
 		    (op == TK_REGISTER && p->op2 == TK_COLLATE)) {
 			coll = sql_get_coll_seq(parse, p->u.zToken, coll_id);
-			*is_found = true;
+			*is_explicit_coll = true;
 			break;
 		}
 		if ((op == TK_AGG_COLUMN || op == TK_COLUMN ||
@@ -190,7 +190,6 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_found, uint32_t *coll_id)
 			if (j >= 0) {
 				coll = sql_column_collation(p->space_def, j,
 							    coll_id);
-				*is_found = true;
 			}
 			break;
 		}
@@ -316,25 +315,62 @@ binaryCompareP5(Expr * pExpr1, Expr * pExpr2, int jumpIfNull)
 	return aff;
 }
 
+int
+collations_are_compatible(uint32_t lhs_id, bool is_lhs_forced,
+			  uint32_t rhs_id, bool is_rhs_forced,
+			  uint32_t *res_id)
+{
+	assert(res_id != NULL);
+	if (is_lhs_forced && is_rhs_forced) {
+		if (lhs_id != rhs_id)
+			return -1;
+	}
+	if (is_lhs_forced) {
+		*res_id = lhs_id;
+		return 0;
+	}
+	if (is_rhs_forced) {
+		*res_id = rhs_id;
+		return 0;
+	}
+	if (lhs_id != rhs_id) {
+		if (lhs_id == COLL_NONE) {
+			*res_id = rhs_id;
+			return 0;
+		}
+		if (rhs_id == COLL_NONE) {
+			*res_id = lhs_id;
+			return 0;
+		}
+		return -1;
+	}
+	*res_id = lhs_id;
+	return 0;
+}
+
 struct coll *
 sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right,
 			    uint32_t *coll_id)
 {
-	struct coll *coll;
-	bool is_found;
 	assert(left != NULL);
-	if ((left->flags & EP_Collate) != 0) {
-		coll = sql_expr_coll(parser, left, &is_found, coll_id);
-	} else if (right != NULL && (right->flags & EP_Collate) != 0) {
-		coll = sql_expr_coll(parser, right, &is_found, coll_id);
-	} else {
-		coll = sql_expr_coll(parser, left, &is_found, coll_id);
-		if (! is_found) {
-			coll = sql_expr_coll(parser, right, &is_found,
-					     coll_id);
-		}
+	bool is_lhs_forced;
+	bool is_rhs_forced;
+	uint32_t lhs_coll_id;
+	uint32_t rhs_coll_id;
+	struct coll *lhs_coll = sql_expr_coll(parser, left, &is_lhs_forced,
+					      &lhs_coll_id);
+	struct coll *rhs_coll = sql_expr_coll(parser, right, &is_rhs_forced,
+					      &rhs_coll_id);
+	if (collations_are_compatible(lhs_coll_id, is_lhs_forced,
+				      rhs_coll_id, is_rhs_forced,
+				      coll_id) != 0) {
+		diag_set(ClientError, ER_ILLEGAL_COLLATION_MIX);
+		parser->rc = SQL_TARANTOOL_ERROR;
+		parser->nErr++;
+		*coll_id = COLL_NONE;
+		return NULL;
 	}
-	return coll;
+	return *coll_id == rhs_coll_id ? rhs_coll : lhs_coll;;
 }
 
 /*
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 1ec92aac7..c3403670b 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1024,7 +1024,7 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 								  regPrev + i);
 						VdbeCoverage(v);
 					}
-					if (is_found) {
+					if (coll != NULL) {
 						sqlite3VdbeChangeP4(v, -1,
 								    (const char *)coll,
 								    P4_COLLSEQ);
@@ -2035,8 +2035,9 @@ sqlite3SelectAddColumnTypeAndCollation(Parse * pParse,		/* Parsing contexts */
 		bool is_found;
 		uint32_t coll_id;
 		if (pTab->def->fields[i].coll_id == COLL_NONE &&
-		    sql_expr_coll(pParse, p, &is_found, &coll_id) && is_found)
+		    sql_expr_coll(pParse, p, &is_found, &coll_id) && coll_id != COLL_NONE)
 			pTab->def->fields[i].coll_id = coll_id;
+		assert(pTab->def->fields[i].coll_id != COLL_BINARY);
 	}
 }
 
@@ -2223,47 +2224,53 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak)
 }
 
 #ifndef SQLITE_OMIT_COMPOUND_SELECT
-static struct coll *
-multi_select_coll_seq_r(Parse *parser, Select *p, int n, bool *is_found,
-			uint32_t *coll_id)
+/**
+ * This function determines resulting collation sequence for
+ * @n-th column of the result set for the compound SELECT
+ * statement. Since compound SELECT performs implicit comparisons
+ * between values, all parts of compound queries must use
+ * the same collation. Otherwise, an error is raised.
+ *
+ * @param parser Parse context.
+ * @param p Select meta-information.
+ * @param n Column number of the result set.
+ * @param is_forced_coll Used if we fall into recursion.
+ *        For most-outer call it is unused. Used to indicate that
+ *        explicit COLLATE clause is used.
+ * @retval Id of collation to be used during string comparison.
+ */
+static uint32_t
+multi_select_coll_seq(struct Parse *parser, struct Select *p, int n,
+		      bool *is_forced_coll)
 {
-	struct coll *coll;
+	bool is_prior_forced = false;
+	bool is_current_forced;
+	uint32_t prior_coll_id = COLL_NONE;
+	uint32_t current_coll_id;
 	if (p->pPrior != NULL) {
-		coll = multi_select_coll_seq_r(parser, p->pPrior, n, is_found,
-					       coll_id);
-	} else {
-		coll = NULL;
-		*coll_id = COLL_NONE;
+		prior_coll_id = multi_select_coll_seq(parser, p->pPrior, n,
+						      &is_prior_forced);
 	}
-	assert(n >= 0);
-	/* iCol must be less than p->pEList->nExpr.  Otherwise an error would
-	 * have been thrown during name resolution and we would not have gotten
-	 * this far
+	/*
+	 * Column number must be less than p->pEList->nExpr.
+	 * Otherwise an error would have been thrown during name
+	 * resolution and we would not have gotten this far.
 	 */
-	if (!*is_found && ALWAYS(n < p->pEList->nExpr)) {
-		coll = sql_expr_coll(parser, p->pEList->a[n].pExpr, is_found,
-				     coll_id);
-	}
-	return coll;
-}
-
-/**
- * The collating sequence for the compound select is taken from the
- * left-most term of the select that has a collating sequence.
- * @param parser Parser.
- * @param p Select.
- * @param n Column number.
- * @param[out] coll_id Collation identifer.
- * @retval The appropriate collating sequence for the n-th column
- *         of the result set for the compound-select statement
- *         "p".
- * @retval NULL The column has no default collating sequence.
- */
-static inline struct coll *
-multi_select_coll_seq(Parse *parser, Select *p, int n, uint32_t *coll_id)
-{
-	bool is_found = false;
-	return multi_select_coll_seq_r(parser, p, n, &is_found, coll_id);
+	assert(n >= 0 && n < p->pEList->nExpr);
+	sql_expr_coll(parser, p->pEList->a[n].pExpr, &is_current_forced,
+		      &current_coll_id);
+	uint32_t res_coll_id;
+	if (collations_are_compatible(prior_coll_id, is_prior_forced,
+				      current_coll_id, is_current_forced,
+				      &res_coll_id) != 0) {
+		diag_set(ClientError, ER_ILLEGAL_COLLATION_MIX);
+		parser->rc = SQL_TARANTOOL_ERROR;
+		parser->nErr++;
+		*is_forced_coll = false;
+		return COLL_NONE;
+	}
+	*is_forced_coll = (is_prior_forced || is_current_forced);
+	return res_coll_id;
 }
 
 /**
@@ -2300,12 +2307,13 @@ sql_multiselect_orderby_to_key_info(struct Parse *parse, struct Select *s,
 		struct ExprList_item *item = &order_by->a[i];
 		struct Expr *term = item->pExpr;
 		uint32_t id;
+		bool unused;
 		if ((term->flags & EP_Collate) != 0) {
-			bool is_found = false;
-			sql_expr_coll(parse, term, &is_found, &id);
+			sql_expr_coll(parse, term, &unused, &id);
 		} else {
-			multi_select_coll_seq(parse, s,
-					      item->u.x.iOrderByCol - 1, &id);
+			id = multi_select_coll_seq(parse, s,
+						   item->u.x.iOrderByCol - 1,
+						   &unused);
 			if (id != COLL_NONE) {
 				const char *name = coll_by_id(id)->name;
 				order_by->a[i].pExpr =
@@ -2947,9 +2955,10 @@ multiSelect(Parse * pParse,	/* Parsing context */
 		struct sql_key_info *key_info = sql_key_info_new(db, nCol);
 		if (key_info == NULL)
 			goto multi_select_end;
+		bool unused;
 		for (int i = 0; i < nCol; i++) {
-			multi_select_coll_seq(pParse, p, i,
-					      &key_info->parts[i].coll_id);
+			key_info->parts[i].coll_id =
+				multi_select_coll_seq(pParse, p, i, &unused);
 		}
 
 		for (struct Select *pLoop = p; pLoop; pLoop = pLoop->pPrior) {
@@ -3374,10 +3383,12 @@ multiSelectOrderBy(Parse * pParse,	/* Parsing context */
 		pParse->nMem += expr_count + 1;
 		sqlite3VdbeAddOp2(v, OP_Integer, 0, regPrev);
 		key_info_dup = sql_key_info_new(db, expr_count);
+		bool unused;
 		if (key_info_dup != NULL) {
 			for (int i = 0; i < expr_count; i++) {
-				multi_select_coll_seq(pParse, p, i,
-					&key_info_dup->parts[i].coll_id);
+				key_info_dup->parts[i].coll_id =
+					multi_select_coll_seq(pParse, p, i,
+							      &unused);
 			}
 		}
 	}
@@ -5351,12 +5362,12 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo)
 			struct ExprList_item *pItem;
 			int j;
 			assert(pList != 0);	/* pList!=0 if pF->pFunc has NEEDCOLL */
-			bool is_found = false;
+			bool unused;
 			uint32_t id;
-			for (j = 0, pItem = pList->a; !is_found && j < nArg;
+			for (j = 0, pItem = pList->a; coll == NULL && j < nArg;
 			     j++, pItem++) {
 				coll = sql_expr_coll(pParse, pItem->pExpr,
-						     &is_found, &id);
+						     &unused, &id);
 			}
 			if (regHit == 0 && pAggInfo->nAccumulator)
 				regHit = ++pParse->nMem;
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 53188e74d..ae82f4643 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4293,13 +4293,14 @@ const char *sqlite3ErrStr(int);
  *
  * @param parse Parsing context.
  * @param expr Expression to scan.
- * @param[out] is_found Flag set if collation was found.
+ * @param[out] is_found Flag set if explicit COLLATE clause is used.
  * @param[out] coll_id Collation identifier.
  *
  * @retval Pointer to collation.
  */
 struct coll *
-sql_expr_coll(Parse * pParse, Expr * pExpr, bool *is_found, uint32_t *coll_id);
+sql_expr_coll(Parse * pParse, Expr * pExpr, bool *is_explicit_coll,
+	      uint32_t *coll_id);
 
 Expr *sqlite3ExprAddCollateToken(Parse * pParse, Expr *, const Token *, int);
 Expr *sqlite3ExprAddCollateString(Parse *, Expr *, const char *);
@@ -4593,6 +4594,29 @@ int sqlite3VdbeParameterIndex(Vdbe *, const char *, int);
 int sqlite3TransferBindings(sqlite3_stmt *, sqlite3_stmt *);
 int sqlite3Reprepare(Vdbe *);
 void sqlite3ExprListCheckLength(Parse *, ExprList *, const char *);
+
+/**
+ * This function verifies that two collations (to be more precise
+ * their ids) are compatible. In terms of SQL ANSI they are
+ * compatible if:
+ *  - one of collations is mentioned alongside with explicit
+ *    COLLATE clause, which forces this collation over another
+ *    one. It is allowed to have the same forced collations;
+ * - both collations are derived from table columns and they
+ *   are the same;
+ * - one collation is derived from table column and another
+ *   one is not specified (i.e. COLL_NONE);
+ * In all other cases they are not accounted to be compatible
+ * and error should be raised.
+ * Collation to be used in comparison operator is returned
+ * via @res_id: in case one of collations is absent, then
+ * the second one is utilized.
+ */
+int
+collations_are_compatible(uint32_t lhs_id, bool is_lhs_forced,
+			  uint32_t rhs_id, bool is_rhs_forced,
+			  uint32_t *res_id);
+
 /**
  * Return a pointer to the collation sequence that should be used
  * by a binary comparison operator comparing left and right.
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 01dcc49cb..b0a9f0651 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -304,11 +304,11 @@ whereScanNext(WhereScan * pScan)
 								Parse *pParse =
 									pWC->pWInfo->pParse;
 								assert(pX->pLeft);
-								uint32_t id;
+								uint32_t unused;
 								struct coll *coll =
 									sql_binary_compare_coll_seq(
 										pParse, pX->pLeft,
-										pX->pRight, &id);
+										pX->pRight, &unused);
 								if (coll != pScan->coll)
 									continue;
 							}
@@ -559,7 +559,7 @@ findIndexCol(Parse * pParse,	/* Parse context */
 			struct coll *coll = sql_expr_coll(pParse,
 							  pList->a[i].pExpr,
 							  &is_found, &id);
-			if (is_found && coll == part_to_match->coll)
+			if (coll == part_to_match->coll)
 				return i;
 		}
 	}
@@ -2289,8 +2289,8 @@ whereRangeVectorLen(Parse * pParse,	/* Parsing context */
 		    sqlite3TableColumnAffinity(space->def, pLhs->iColumn);
 		if (aff != idxaff)
 			break;
-		uint32_t id;
-		pColl = sql_binary_compare_coll_seq(pParse, pLhs, pRhs, &id);
+		uint32_t unused;
+		pColl = sql_binary_compare_coll_seq(pParse, pLhs, pRhs, &unused);
 		if (pColl == 0)
 			break;
 		if (idx_def->key_def->parts[i + nEq].coll != pColl)
@@ -3409,8 +3409,7 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
 								      &is_found, &id);
 						struct coll *idx_coll =
 							idx_def->key_def->parts[j].coll;
-						if (is_found &&
-						    coll != idx_coll)
+						if (coll != idx_coll)
 							continue;
 					}
 					isMatch = 1;
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 9fa6ce15d..971f428b7 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -168,7 +168,7 @@ exprCommute(Parse * pParse, Expr * pExpr)
 			bool is_found;
 			uint32_t id;
 			sql_expr_coll(pParse, pExpr->pLeft, &is_found, &id);
-			if (is_found) {
+			if (id != COLL_NONE) {
 				/*
 				 * Neither X nor Y have COLLATE
 				 * operators, but X has a
@@ -837,15 +837,16 @@ termIsEquivalence(Parse * pParse, Expr * pExpr)
 	    ) {
 		return 0;
 	}
-	uint32_t id;
+	uint32_t unused;
 	struct coll *coll1 =
 	    sql_binary_compare_coll_seq(pParse, pExpr->pLeft, pExpr->pRight,
-					&id);
+					&unused);
 	if (coll1 == NULL)
 		return 1;
-	bool unused;
-	coll1 = sql_expr_coll(pParse, pExpr->pLeft, &unused, &id);
-	struct coll *coll2 = sql_expr_coll(pParse, pExpr->pRight, &unused, &id);
+	bool unused1;
+	coll1 = sql_expr_coll(pParse, pExpr->pLeft, &unused1, &unused);
+	struct coll *coll2 = sql_expr_coll(pParse, pExpr->pRight, &unused1,
+					   &unused);
 	return coll1 != NULL && coll1 == coll2;
 }
 
diff --git a/test/box/misc.result b/test/box/misc.result
index 3d7317caf..e8c87db68 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -497,6 +497,7 @@ t;
   168: box.error.DROP_FK_CONSTRAINT
   169: box.error.NO_SUCH_CONSTRAINT
   170: box.error.CONSTRAINT_EXISTS
+  171: box.error.ILLEGAL_COLLATION_MIX
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index c8ad9039d..44b45461b 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(541)
+test:plan(539)
 
 --!./tcltestrunner.lua
 -- 2010 July 16
@@ -1690,12 +1690,10 @@ test:do_select_tests(
         {"1", "SELECT 'abc'                UNION SELECT 'ABC'", {"ABC",  "abc"}},
         {"2", "SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC'", {"ABC" }},
         {"3", "SELECT 'abc'                UNION SELECT 'ABC' COLLATE \"unicode_ci\"", {"ABC" }},
-        {"4", "SELECT 'abc' COLLATE binary UNION SELECT 'ABC' COLLATE \"unicode_ci\"", {"ABC",  "abc"}},
-        {"5", "SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC' COLLATE binary", {"ABC" }},
-        {"6", "SELECT a FROM y1 UNION SELECT b FROM y1", {"abc" }},
-        {"7", "SELECT b FROM y1 UNION SELECT a FROM y1", {"Abc",  "abc"}},
-        {"8", "SELECT a FROM y1 UNION SELECT c FROM y1", {"aBC" }},
-        {"9", "SELECT a FROM y1 UNION SELECT c COLLATE binary FROM y1", {"aBC" }},
+        {"4", "SELECT 'abc' COLLATE binary UNION SELECT 'ABC'", {"ABC",  "abc"}},
+        {"5", "SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC'", {"ABC" }},
+        {"6", "SELECT a FROM y1 UNION SELECT c FROM y1", {"aBC" }},
+        {"7", "SELECT a FROM y1 UNION SELECT c COLLATE binary FROM y1", {"Abc", "aBC" }},
     })
 
 -- EVIDENCE-OF: R-32706-07403 No affinity transformations are applied to
diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua
index ac39c5fca..3ea0da653 100755
--- a/test/sql-tap/in4.test.lua
+++ b/test/sql-tap/in4.test.lua
@@ -531,7 +531,7 @@ test:do_execsql_test(
         SELECT c FROM t4a WHERE a=b ORDER BY c;
     ]], {
         -- <in4-4.1>
-        3
+        1, 3
         -- </in4-4.1>
     })
 
diff --git a/test/sql-tap/join.test.lua b/test/sql-tap/join.test.lua
index ac05a98b2..dcd1fbd4c 100755
--- a/test/sql-tap/join.test.lua
+++ b/test/sql-tap/join.test.lua
@@ -1003,7 +1003,7 @@ test:do_execsql_test(
         SELECT a,b FROM t2 NATURAL JOIN t1 
     ]], {
         -- <join-11.7>
-        "two", 2
+        "one", 1, "two", 2
         -- </join-11.7>
     })
 
diff --git a/test/sql-tap/tkt3493.test.lua b/test/sql-tap/tkt3493.test.lua
index 85f3c089f..c074d9446 100755
--- a/test/sql-tap/tkt3493.test.lua
+++ b/test/sql-tap/tkt3493.test.lua
@@ -246,7 +246,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "tkt3493-3.1",
     [[
-        CREATE TABLE t2(a COLLATE "unicode_ci" PRIMARY KEY, b COLLATE BINARY);
+        CREATE TABLE t2(a COLLATE "unicode_ci" PRIMARY KEY, b);
         INSERT INTO t2 VALUES('aBc', 'DeF');
     ]], {
         -- <tkt3493-3.1>
@@ -304,7 +304,7 @@ test:do_execsql_test(
         SELECT b>a FROM t2 GROUP BY a, b
     ]], {
         -- <tkt3493-3.3.3>
-        0
+        1
         -- </tkt3493-3.3.3>
     })
 
diff --git a/test/sql-tap/transitive1.test.lua b/test/sql-tap/transitive1.test.lua
index ed3238f04..3c0a36ba9 100755
--- a/test/sql-tap/transitive1.test.lua
+++ b/test/sql-tap/transitive1.test.lua
@@ -503,7 +503,7 @@ test:do_execsql_test(
         SELECT * FROM c1 WHERE x=y AND z=y AND z='abc';
     ]], {
         -- <transitive1-570>
-
+        1, "ABC", "ABC", "abc"
         -- </transitive1-570>
     })
 
@@ -516,7 +516,7 @@ test:do_execsql_test(
         SELECT * FROM c1 WHERE x=y AND z=y AND z='abc';
     ]], {
         -- <transitive1-570eqp>
-        "/SEARCH TABLE C1 USING COVERING INDEX C1X/"
+        "/SCAN TABLE C1/"
         -- </transitive1-570eqp>
     })
 
diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua
index c6a895875..9c04cf2a6 100755
--- a/test/sql-tap/with1.test.lua
+++ b/test/sql-tap/with1.test.lua
@@ -880,7 +880,7 @@ test:do_execsql_test("10.8.4.2", [[
   SELECT a FROM tst UNION ALL SELECT b COLLATE "unicode_ci" FROM tst ORDER BY 1;
 ]], {
   -- <10.8.4.2>
-  "A", "B", "C", "a", "b", "c"
+  "a", "A", "b", "B", "c", "C"
   -- </10.8.4.2>
 })
 
diff --git a/test/sql/collation.result b/test/sql/collation.result
index f6254f866..c5934d4e2 100644
--- a/test/sql/collation.result
+++ b/test/sql/collation.result
@@ -123,6 +123,69 @@ box.space.T:format()[3]['collation']
 box.sql.execute("DROP TABLE t;")
 ---
 ...
+-- gh-3185: collations of LHS and RHS must be compatible.
+--
+box.sql.execute("CREATE TABLE t (id INT PRIMARY KEY, a TEXT, b TEXT COLLATE BINARY, c TEXT COLLATE \"unicode_ci\");")
+---
+...
+box.sql.execute("SELECT * FROM t WHERE a = b;")
+---
+- []
+...
+box.sql.execute("SELECT * FROM t WHERE a COLLATE BINARY = b;")
+---
+- []
+...
+box.sql.execute("SELECT * FROM t WHERE b = c;")
+---
+- error: Illegal mix of collations
+...
+box.sql.execute("SELECT * FROM t WHERE b COLLATE BINARY = c;")
+---
+- []
+...
+box.sql.execute("SELECT * FROM t WHERE a = c;")
+---
+- []
+...
+box.sql.execute("SELECT * FROM t WHERE a COLLATE BINARY = c COLLATE \"unicode\";")
+---
+- error: Illegal mix of collations
+...
+-- Compound queries perform implicit comparisons between values.
+-- Hence, rules for collations compatibilities are the same.
+--
+box.sql.execute("SELECT 'abc' COLLATE binary UNION SELECT 'ABC' COLLATE \"unicode_ci\"")
+---
+- error: Illegal mix of collations
+...
+box.sql.execute("SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC' COLLATE binary")
+---
+- error: Illegal mix of collations
+...
+box.sql.execute("SELECT c FROM t UNION SELECT b FROM t;")
+---
+- error: Illegal mix of collations
+...
+box.sql.execute("SELECT b FROM t UNION SELECT a FROM t;")
+---
+- []
+...
+box.sql.execute("SELECT a FROM t UNION SELECT c FROM t;")
+---
+- []
+...
+box.sql.execute("SELECT c COLLATE BINARY FROM t UNION SELECT a FROM t;")
+---
+- []
+...
+box.sql.execute("SELECT b COLLATE \"unicode\" FROM t UNION SELECT a FROM t;")
+---
+- []
+...
+box.sql.execute("DROP TABLE t;")
+---
+...
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 ---
 ...
diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua
index f9d653717..decab2560 100644
--- a/test/sql/collation.test.lua
+++ b/test/sql/collation.test.lua
@@ -50,4 +50,26 @@ box.space.T:format()[2]['collation']
 box.space.T:format()[3]['collation']
 box.sql.execute("DROP TABLE t;")
 
+-- gh-3185: collations of LHS and RHS must be compatible.
+--
+box.sql.execute("CREATE TABLE t (id INT PRIMARY KEY, a TEXT, b TEXT COLLATE BINARY, c TEXT COLLATE \"unicode_ci\");")
+box.sql.execute("SELECT * FROM t WHERE a = b;")
+box.sql.execute("SELECT * FROM t WHERE a COLLATE BINARY = b;")
+box.sql.execute("SELECT * FROM t WHERE b = c;")
+box.sql.execute("SELECT * FROM t WHERE b COLLATE BINARY = c;")
+box.sql.execute("SELECT * FROM t WHERE a = c;")
+box.sql.execute("SELECT * FROM t WHERE a COLLATE BINARY = c COLLATE \"unicode\";")
+
+-- Compound queries perform implicit comparisons between values.
+-- Hence, rules for collations compatibilities are the same.
+--
+box.sql.execute("SELECT 'abc' COLLATE binary UNION SELECT 'ABC' COLLATE \"unicode_ci\"")
+box.sql.execute("SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC' COLLATE binary")
+box.sql.execute("SELECT c FROM t UNION SELECT b FROM t;")
+box.sql.execute("SELECT b FROM t UNION SELECT a FROM t;")
+box.sql.execute("SELECT a FROM t UNION SELECT c FROM t;")
+box.sql.execute("SELECT c COLLATE BINARY FROM t UNION SELECT a FROM t;")
+box.sql.execute("SELECT b COLLATE \"unicode\" FROM t UNION SELECT a FROM t;")
+
+box.sql.execute("DROP TABLE t;")
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
-- 
2.15.1