Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 1/2] sql: refactor sql_expr_coll and sql_binary_compare_coll_seq functions
Date: Fri, 15 Feb 2019 02:26:21 +0300	[thread overview]
Message-ID: <7E6C9742-DC6B-47FC-B594-B897E3EEA1F8@tarantool.org> (raw)
In-Reply-To: <07f58a45-5cf2-e848-78f1-c8a67028733c@tarantool.org>


> On 16/01/2019 16:13, Nikita Pettik wrote:
>> Lets make sql_expr_coll() return error code for two reasons. Firstly,
>> we are going to use this function to detect operands of concatenation
>> with incompatible collations. Secondly, pointer to struct coll in most
>> cases is redundant since collation id (which in turn returned via output
>> parameter) is enough to proceed the same operations.
>> For the same reason lets make sql_binary_compare_coll_seq() return
>> collation id instead of struct coll* and remove corresponding output
>> parameter.
> 
> First of all, I agree with Kostja - it does not help in anything that
> you removed coll * from output parameters/return value. It just slows
> down some places like codeCompare, sqlite3ExprCodeIN, sqlite3ExprCodeTarget
> etc. - you just do coll_by_id() twice.
> Moreover, maybe making some
> places more accurate and compact, it pads out other ones and involves
> additional headers like in whereexpr.c. Please, return as it was.

Ok, then I will move struct coll * to output params for sql_expr_coll().
As for sql_binary_compare_coll_seq(), struct coll is needed only once -
in codeCompare(); other 5 usages of this function leads to throwing away
struct coll. In this case I believe it is really reasonable to operate only
with id. Anyway, id moved to output params, now it returns error code.

>> Needed for #3947
>> ---
>>  src/box/sql/expr.c      | 82 ++++++++++++++++++++++++-------------------------
>>  src/box/sql/select.c    | 27 ++++++++++------
>>  src/box/sql/sqliteInt.h | 14 ++++-----
>>  src/box/sql/where.c     | 56 ++++++++++++++++-----------------
>>  src/box/sql/whereexpr.c | 23 ++++++++------
>>  5 files changed, 105 insertions(+), 97 deletions(-)
>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
>> index b67b22c23..f8819f779 100644
>> --- a/src/box/sql/expr.c
>> +++ b/src/box/sql/expr.c
>> @@ -205,7 +204,7 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, 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);
>> +			(void) sql_get_coll_seq(parse, p->u.zToken, coll_id);
> 
> 1. It is not necessary to prefix (void) each function result of which
> you ignore. By the way, I do not understand why do you ignore it. Here
> and in all other places.

Yep, it is not necessary, but this way is more descriptive.
Refactored whole function, so it is not longer ignored.

>> @@ -377,27 +376,28 @@ illegal_collation_mix:
>>  	return -1;
>>  }
>>  -struct coll *
>> -sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right,
>> -			    uint32_t *coll_id)
>> +uint32_t
>> +sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right)
>>  {
>>  	assert(left != NULL);
>>  	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 (sql_expr_coll(parser, left, &is_lhs_forced, &lhs_coll_id) != 0)
>> +		goto err;
>> +	if (sql_expr_coll(parser, right, &is_rhs_forced, &rhs_coll_id) != 0)
>> +		goto err;
>> +	uint32_t coll_id;
>>  	if (collations_check_compatibility(lhs_coll_id, is_lhs_forced,
>>  					   rhs_coll_id, is_rhs_forced,
>> -					   coll_id) != 0) {
>> -		parser->rc = SQL_TARANTOOL_ERROR;
>> -		parser->nErr++;
>> -		return NULL;
>> -	}
>> -	return *coll_id == rhs_coll_id ? rhs_coll : lhs_coll;;
>> +					   &coll_id) != 0)
>> +		goto err;
>> +	return coll_id;
>> +err:
>> +	parser->rc = SQL_TARANTOOL_ERROR;
>> +	parser->nErr++;
>> +	return COLL_NONE;
> 
> 2. Please, do not return some obscure error codes - we use only 0/-1
> and NULL/not NULL almost everywhere except some archaic or misreviewed
> code. If you want to return an error, it is better to move coll_id to
> out parameters.

Ok, that’s what I did. See diff below.

>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index 7501fadc8..50d297815 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -4337,10 +4337,10 @@ const char *sqlite3ErrStr(int);
>>   *             clause is used.
>>   * @param[out] coll_id Collation identifier.
>>   *
>> - * @retval Pointer to collation.
>> + * @retval Return code: < 0 in case of error, 0 on success.
> 
> 3. Please, use separate @retval for each value.

As you wish. It is not matter of discussion, but this
rule seems to be inconsistent: somewhere we use
one retval clause for all possible return values;
in other places - several ones.

>>   */
>> -struct coll *
>> -sql_expr_coll(Parse * pParse, Expr * pExpr, bool *is_explicit_coll,
>> +int
>> +sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll,
>>  	      uint32_t *coll_id);
>>    Expr *sqlite3ExprAddCollateToken(Parse * pParse, Expr *, const Token *, int);
>> @@ -4684,13 +4684,11 @@ collations_check_compatibility(uint32_t lhs_id, bool is_lhs_forced,
>>   * @param parser Parser.
>>   * @param left Left expression.
>>   * @param right Right expression. Can be NULL.
>> - * @param[out] coll_id Collation identifier.
>>   *
>> - * @retval Collation object.
>> + * @retval Id of collation object.
> 
> 4. You didn't mentioned that it can be COLL_NONE, that means an error.

Skipped this comment due to refactoring of this function.

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a367a07d4..5617e0f62 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -193,10 +193,13 @@ sqlExprSkipCollate(Expr * pExpr)
 }
 
 int
-sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id)
+sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
+             struct coll **coll)
 {
+       assert(coll != NULL);
        *is_explicit_coll = false;
        *coll_id = COLL_NONE;
+       *coll = NULL;
        while (p != NULL) {
                int op = p->op;
                if (op == TK_CAST || op == TK_UPLUS) {
@@ -205,7 +208,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id)
                }
                if (op == TK_COLLATE ||
                    (op == TK_REGISTER && p->op2 == TK_COLLATE)) {
-                       (void) sql_get_coll_seq(parse, p->u.zToken, coll_id);
+                       *coll = sql_get_coll_seq(parse, p->u.zToken, coll_id);
+                       if (coll == NULL)
+                               return -1;
                        *is_explicit_coll = true;
                        break;
                }
@@ -217,8 +222,8 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id)
                         */
                        int j = p->iColumn;
                        if (j >= 0) {
-                               (void) sql_column_collation(p->space_def, j,
-                                                           coll_id);
+                               *coll = sql_column_collation(p->space_def, j,
+                                                            coll_id);
                        }
                        break;
                }
@@ -335,28 +340,30 @@ illegal_collation_mix:
        return -1;
 }
 
-uint32_t
-sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right)
+int
+sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right,
+                           uint32_t *id)
 {
        assert(left != NULL);
+       assert(id != NULL);
        bool is_lhs_forced;
        bool is_rhs_forced;
        uint32_t lhs_coll_id;
        uint32_t rhs_coll_id;
-       if (sql_expr_coll(parser, left, &is_lhs_forced, &lhs_coll_id) != 0)
-               goto err;
-       if (sql_expr_coll(parser, right, &is_rhs_forced, &rhs_coll_id) != 0)
-               goto err;
-       uint32_t coll_id;
+       struct coll *unused;
+       if (sql_expr_coll(parser, left, &is_lhs_forced, &lhs_coll_id,
+                         &unused) != 0)
+               return -1;
+       if (sql_expr_coll(parser, right, &is_rhs_forced, &rhs_coll_id,
+                         &unused) != 0)
+               return -1;
        if (collations_check_compatibility(lhs_coll_id, is_lhs_forced,
-                                          rhs_coll_id, is_rhs_forced,
-                                          &coll_id) != 0)
-               goto err;
-       return coll_id;
-err:
-       parser->rc = SQL_TARANTOOL_ERROR;
-       parser->nErr++;
-       return COLL_NONE;
+                                          rhs_coll_id, is_rhs_forced, id) != 0) {
+               parser->rc = SQL_TARANTOOL_ERROR;
+               parser->nErr++;
+               return -1;
+       }
+       return 0;
 }
 
 /*
@@ -372,7 +379,9 @@ codeCompare(Parse * pParse, /* The parsing (and code generating) context */
            int jumpIfNull      /* If true, jump if either operand is NULL */
     )
 {
-       uint32_t id = sql_binary_compare_coll_seq(pParse, pLeft, pRight);
+       uint32_t id;
+       if (sql_binary_compare_coll_seq(pParse, pLeft, pRight, &id) != 0)
+               return -1;
        struct coll *coll = coll_by_id(id)->coll;
        int p5 = binaryCompareP5(pLeft, pRight, jumpIfNull);
        int addr = sqlVdbeAddOp4(pParse->pVdbe, opcode, in2, dest, in1,
@@ -2428,8 +2437,9 @@ sqlFindInIndex(Parse * pParse,    /* Parsing context */
                                for (i = 0; i < nExpr; i++) {
                                        Expr *pLhs = sqlVectorFieldSubexpr(pX->pLeft, i);
                                        Expr *pRhs = pEList->a[i].pExpr;
-                                       uint32_t id =
-                                               sql_binary_compare_coll_seq(pParse, pLhs, pRhs);
+                                       uint32_t id;
+                                       if (sql_binary_compare_coll_seq(pParse, pLhs, pRhs, &id) != 0)
+                                               break;
                                        int j;
 
                                        for (j = 0; j < nExpr; j++) {
@@ -2749,9 +2759,9 @@ sqlCodeSubselect(Parse * pParse,  /* Parsing context */
                                                Expr *p =
                                                    sqlVectorFieldSubexpr
                                                    (pLeft, i);
-                                               key_info->parts[i].coll_id =
-                                                       sql_binary_compare_coll_seq(pParse, p,
-                                                                                   pEList->a[i].pExpr);
+                                               if (sql_binary_compare_coll_seq(pParse, p, pEList->a[i].pExpr,
+                                                                               &key_info->parts[i].coll_id) != 0)
+                                                       return 0;
                                        }
                                }
                        } else if (ALWAYS(pExpr->x.pList != 0)) {
@@ -2770,9 +2780,11 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */
                                enum field_type lhs_type =
                                        sql_expr_type(pLeft);
                                bool unused;
-                               (void) sql_expr_coll(pParse, pExpr->pLeft,
-                                                    &unused,
-                                                    &key_info->parts[0].coll_id);
+                               struct coll *unused_coll;
+                               if (sql_expr_coll(pParse, pExpr->pLeft, &unused,
+                                                 &key_info->parts[0].coll_id,
+                                                 &unused_coll) != 0)
+                                       return 0;
 
                                /* Loop through each expression in <exprlist>. */
                                r1 = sqlGetTempReg(pParse);
@@ -3035,9 +3047,10 @@ sqlExprCodeIN(Parse * pParse,    /* Parsing and code generating context */
                bool unused;
                uint32_t id;
                ExprList *pList = pExpr->x.pList;
-               if (sql_expr_coll(pParse, pExpr->pLeft, &unused, &id) != 0)
+               struct coll *coll;
+               if (sql_expr_coll(pParse, pExpr->pLeft, &unused, &id,
+                                 &coll) != 0)
                        goto sqlExprCodeIN_finished;
-               struct coll *coll = coll_by_id(id)->coll;
                int labelOk = sqlVdbeMakeLabel(v);
                int r2, regToFree;
                int regCkNull = 0;
@@ -3169,9 +3182,9 @@ sqlExprCodeIN(Parse * pParse,     /* Parsing and code generating context */
                uint32_t id;
                int r3 = sqlGetTempReg(pParse);
                Expr *p = sqlVectorFieldSubexpr(pLeft, i);
-               if (sql_expr_coll(pParse, p, &unused, &id) != 0)
+               struct coll *pColl;
+               if (sql_expr_coll(pParse, p, &unused, &id, &pColl) != 0)
                        goto sqlExprCodeIN_finished;
-               struct coll *pColl = coll_by_id(id)->coll;
                /* Tarantool: Replace i -> aiMap [i], since original order of columns
                 * is preserved.
                 */
@@ -4001,9 +4014,9 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
                                        uint32_t id;
                                        if (sql_expr_coll(pParse,
                                                          pFarg->a[i].pExpr,
-                                                         &unused, &id) != 0)
-                                               break;
-                                       coll = coll_by_id(id)->coll;;
+                                                         &unused, &id,
+                                                         &coll) != 0)
+                                               return 0;
                                }
                        }
                        if (pFarg) {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index aed40455b..926f5d011 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1042,11 +1042,12 @@ selectInnerLoop(Parse * pParse,         /* The parser context */
                                for (i = 0; i < nResultCol; i++) {
                                        bool is_found;
                                        uint32_t id;
+                                       struct coll *coll;
                                        if (sql_expr_coll(pParse,
                                                          pEList->a[i].pExpr,
-                                                         &is_found, &id) != 0)
+                                                         &is_found, &id,
+                                                         &coll) != 0)
                                                break;
-                                       struct coll *coll = coll_by_id(id)->coll;
                                        if (i < nResultCol - 1) {
                                                sqlVdbeAddOp3(v, OP_Ne,
                                                                  regResult + i,
@@ -1428,7 +1429,9 @@ sql_expr_list_to_key_info(struct Parse *parse, struct ExprList *list, int start)
                struct key_part_def *part = &key_info->parts[i - start];
                bool unused;
                uint32_t id;
-               if (sql_expr_coll(parse, item->pExpr, &unused, &id) != 0) {
+               struct coll *unused_coll;
+               if (sql_expr_coll(parse, item->pExpr, &unused, &id,
+                                 &unused_coll) != 0) {
                        sqlDbFree(parse->db, key_info);
                        return NULL;
                }
@@ -1970,10 +1973,10 @@ sqlSelectAddColumnTypeAndCollation(Parse * pParse,              /* Parsing contexts */
                pTab->def->fields[i].type = sql_expr_type(p);
                bool is_found;
                uint32_t coll_id;
-
+               struct coll *unused;
                if (pTab->def->fields[i].coll_id == COLL_NONE &&
-                   sql_expr_coll(pParse, p, &is_found, &coll_id) == 0 &&
-                   coll_id != COLL_NONE)
+                   sql_expr_coll(pParse, p, &is_found, &coll_id,
+                                 &unused) == 0 && coll_id != COLL_NONE)
                        pTab->def->fields[i].coll_id = coll_id;
        }
 }
@@ -2221,9 +2224,10 @@ multi_select_coll_seq_r(struct Parse *parser, struct Select *p, int n,
         * resolution and we would not have got this far.
         */
        assert(n >= 0 && n < p->pEList->nExpr);
+       struct coll *unused;
        if (sql_expr_coll(parser, p->pEList->a[n].pExpr, &is_current_forced,
-                         &current_coll_id) != 0)
-               return COLL_NONE;
+                         &current_coll_id, &unused) != 0)
+               return 0;
        uint32_t res_coll_id;
        if (collations_check_compatibility(prior_coll_id, is_prior_forced,
                                           current_coll_id, is_current_forced,
@@ -2279,7 +2283,10 @@ sql_multiselect_orderby_to_key_info(struct Parse *parse, struct Select *s,
                uint32_t id;
                bool unused;
                if ((term->flags & EP_Collate) != 0) {
-                       (void) sql_expr_coll(parse, term, &unused, &id);
+                       struct coll *unused_coll;
+                       if (sql_expr_coll(parse, term, &unused, &id,
+                                         &unused_coll) != 0)
+                               return 0;
                } else {
                        id = multi_select_coll_seq(parse, s,
                                                   item->u.x.iOrderByCol - 1);
@@ -5357,9 +5364,8 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo)
                        for (j = 0, pItem = pList->a; coll == NULL && j < nArg;
                             j++, pItem++) {
                                if (sql_expr_coll(pParse, pItem->pExpr,
-                                                 &unused, &id) != 0)
+                                                 &unused, &id, &coll) != 0)
                                        return;
-                               coll = coll_by_id(id)->coll;
                        }
                        if (regHit == 0 && pAggInfo->nAccumulator)
                                regHit = ++pParse->nMem;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 87cdbbcc1..70f971f8c 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4344,12 +4344,14 @@ const char *sqlErrStr(int);
  * @param[out] is_explicit_coll Flag set if explicit COLLATE
  *             clause is used.
  * @param[out] coll_id Collation identifier.
+ * @param[out] coll Collation object.
  *
- * @retval Return code: < 0 in case of error, 0 on success.
+ * @retval 0 on success.
+ * @retval -1 in case of error.
  */
 int
-sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll,
-             uint32_t *coll_id);
+sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
+             struct coll **coll);
 
 Expr *sqlExprAddCollateToken(Parse * pParse, Expr *, const Token *, int);
 Expr *sqlExprAddCollateString(Parse *, Expr *, const char *);
@@ -4681,11 +4683,14 @@ collations_check_compatibility(uint32_t lhs_id, bool is_lhs_forced,
  * @param parser Parser.
  * @param left Left expression.
  * @param right Right expression. Can be NULL.
+ * @param[out] id Id of resulting collation.
  *
- * @retval Id of collation object.
+ * @retval 0 on success.
+ * @retval -1 in case of error (e.g. no collation found).
  */
-uint32_t
-sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right);
+int
+sql_binary_compare_coll_seq(Parse *parser, Expr *left, Expr *right,
+                           uint32_t *id);
 int sqlTempInMemory(const sql *);
 #ifndef SQL_OMIT_CTE
 With *sqlWithAdd(Parse *, With *, Token *, ExprList *, Select *);
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index f27de5dd7..631a5feba 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -306,10 +306,12 @@ whereScanNext(WhereScan * pScan)
                                                                Parse *pParse =
                                                                        pWC->pWInfo->pParse;
                                                                assert(pX->pLeft);
-                                                               uint32_t id =
-                                                                       sql_binary_compare_coll_seq(
-                                                                               pParse, pX->pLeft,
-                                                                               pX->pRight);
+                                                               uint32_t id;
+                                                               if (sql_binary_compare_coll_seq(
+                                                                       pParse, pX->pLeft,
+                                                                       pX->pRight, &id) != 0)
+                                                                       break;
                                                                struct coll *coll =
                                                                        id != COLL_NONE ?
                                                                        coll_by_id(id)->coll :
@@ -559,8 +561,9 @@ findIndexCol(Parse * pParse,        /* Parse context */
                    p->iColumn == (int) part_to_match->fieldno) {
                        bool is_found;
                        uint32_t id;
+                       struct coll *unused;
                        if (sql_expr_coll(pParse, pList->a[i].pExpr,
-                                         &is_found, &id) != 0)
+                                         &is_found, &id, &unused) != 0)
                                return -1;
                        if (id == part_to_match->coll_id)
                                return i;
@@ -2280,7 +2283,9 @@ whereRangeVectorLen(Parse * pParse,       /* Parsing context */
                        space->def->fields[pLhs->iColumn].type : FIELD_TYPE_INTEGER;
                if (type != idx_type)
                        break;
-               uint32_t id = sql_binary_compare_coll_seq(pParse, pLhs, pRhs);
+               uint32_t id;
+               if (sql_binary_compare_coll_seq(pParse, pLhs, pRhs, &id) != 0)
+                       break;
                if (id == COLL_NONE)
                        break;
                if (idx_def->key_def->parts[i + nEq].coll_id != id)
@@ -3264,13 +3269,14 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,   /* The WHERE clause */
                                bool unused;
                                uint32_t lhs_id;
                                uint32_t rhs_id;
+                               struct coll *unused_coll;
                                if (sql_expr_coll(pWInfo->pParse,
                                                  pOrderBy->a[i].pExpr, &unused,
-                                                 &lhs_id) != 0)
+                                                 &lhs_id, &unused_coll) != 0)
                                        return 0;
                                if (sql_expr_coll(pWInfo->pParse,
                                                  pTerm->pExpr, &unused,
-                                                 &rhs_id) != 0)
+                                                 &rhs_id, &unused_coll) != 0)
                                        return 0;
                                if (lhs_id != rhs_id)
                                        continue;
@@ -3395,9 +3401,11 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,    /* The WHERE clause */
                                        if (iColumn >= 0) {
                                                bool is_found;
                                                uint32_t id;
-                                               sql_expr_coll(pWInfo->pParse,
-                                                             pOrderBy->a[i].pExpr,
-                                                             &is_found, &id);
+                                               struct coll *unused;
+                                               if (sql_expr_coll(pWInfo->pParse,
+                                                                 pOrderBy->a[i].pExpr,
+                                                                 &is_found, &id, &unused) != 0)
+                                                       return 0;
                                                if (idx_def->key_def->parts[j].coll_id != id)
                                                        continue;
                                        }
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 758c92896..703980f02 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -168,8 +168,10 @@ exprCommute(Parse * pParse, Expr * pExpr)
                } else {
                        bool is_found;
                        uint32_t id;
-                       (void) sql_expr_coll(pParse, pExpr->pLeft, &is_found,
-                                            &id);
+                       struct coll *unused;
+                       if (sql_expr_coll(pParse, pExpr->pLeft, &is_found, &id,
+                                         &unused) != 0)
+                               return;
                        if (id != COLL_NONE) {
                                /*
                                 * Neither X nor Y have COLLATE
@@ -849,16 +851,21 @@ termIsEquivalence(Parse * pParse, Expr * pExpr)
        if (lhs_type != rhs_type && (!sql_type_is_numeric(lhs_type) ||
                                     !sql_type_is_numeric(rhs_type)))
                return 0;
-       uint32_t id = sql_binary_compare_coll_seq(pParse, pExpr->pLeft,
-                                                 pExpr->pRight);
+       uint32_t id;
+       if (sql_binary_compare_coll_seq(pParse, pExpr->pLeft, pExpr->pRight,
+                                       &id) != 0)
+               return 0;
        if (id == COLL_NONE)
                return 1;
        bool unused1;
        uint32_t lhs_id;
        uint32_t rhs_id;
-       if (sql_expr_coll(pParse, pExpr->pLeft, &unused1, &lhs_id) != 0)
+       struct coll *unused;
+       if (sql_expr_coll(pParse, pExpr->pLeft, &unused1, &lhs_id,
+                         &unused) != 0)
                return 0;
-       if (sql_expr_coll(pParse, pExpr->pRight, &unused1, &rhs_id) != 0)
+       if (sql_expr_coll(pParse, pExpr->pRight, &unused1, &rhs_id,
+                         &unused) != 0)
                return 0;
        return lhs_id != COLL_NONE && lhs_id == rhs_id;
 }

  reply	other threads:[~2019-02-14 23:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-16 13:13 [tarantool-patches] [PATCH 0/2] Compute derived collation for concatenation Nikita Pettik
2019-01-16 13:13 ` [tarantool-patches] [PATCH 1/2] sql: refactor sql_expr_coll and sql_binary_compare_coll_seq functions Nikita Pettik
2019-01-17 13:28   ` [tarantool-patches] " Konstantin Osipov
2019-01-24 18:28   ` Vladislav Shpilevoy
2019-02-14 23:26     ` n.pettik [this message]
2019-01-16 13:13 ` [tarantool-patches] [PATCH 2/2] sql: compute resulting collation for concatenation Nikita Pettik
2019-01-17 13:33   ` [tarantool-patches] " Konstantin Osipov
2019-01-17 19:19     ` n.pettik
2019-01-18  9:59     ` Kirill Yukhin
2019-01-24 18:29   ` Vladislav Shpilevoy
2019-02-14 23:26     ` n.pettik
2019-02-22 11:23       ` Vladislav Shpilevoy
2019-02-22 13:40         ` n.pettik
2019-02-22 13:51           ` Vladislav Shpilevoy
2019-02-25 11:29 ` [tarantool-patches] Re: [PATCH 0/2] Compute derived " Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7E6C9742-DC6B-47FC-B594-B897E3EEA1F8@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH 1/2] sql: refactor sql_expr_coll and sql_binary_compare_coll_seq functions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox