From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id D6FDD24FCF for ; Fri, 1 Feb 2019 11:39:06 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wDIUCf0qraZl for ; Fri, 1 Feb 2019 11:39:06 -0500 (EST) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 7284E23EBA for ; Fri, 1 Feb 2019 11:39:06 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH 6/8] sql: replace affinity with field type in struct Expr From: "n.pettik" In-Reply-To: Date: Fri, 1 Feb 2019 19:39:04 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <3256389D-8BEA-4FB2-B214-D4F66067F333@tarantool.org> References: <406a1cab-d3cd-437b-668e-141697fbd637@tarantool.org> <8D9E26B4-FC3D-4EBD-9941-5570A5ECFA17@tarantool.org> <2837d5ad-fc59-8c3e-0fef-b82ea764f6ca@tarantool.org> <7FD8757A-713C-4887-9268-42849FFEE3A8@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy >>>>> 7. Why did not you move this conversion to sql_expr_type? I mean >>>>> ANY -> SCALAR. >>>> Unfortunately, right now I can=E2=80=99t replace it with ANY. It is = still used >>>> when IN operator comes with one operand (see EP_Generic flag). >>>> I have already sent patch which removes it: >>>> = https://github.com/tarantool/tarantool/tree/np/gh-3934-IN-operator-fix >>>=20 >>> I've already acked the patch. Please, hurry up Kirill Y. to push it, >>> rebase this branch, and replace ANY with SCALAR. >> I had to add some other fixes to make this happen: >> Changes to sql: replace affinity with field type for func: >> diff --git a/test-run b/test-run >> index 02207efd2..4c3f11812 160000 >> --- a/test-run >> +++ b/test-run >> @@ -1 +1 @@ >> -Subproject commit 02207efd2ca44067b76c79bf012f142016d929ae >> +Subproject commit 4c3f11812c0c17f08780f31b6a748eef1126b2e2 >=20 > 1. ??? Accidentally trapped to diff. >> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >> index fdf00273a..e1a0dfa73 100644 >> --- a/src/box/sql/expr.c >> +++ b/src/box/sql/expr.c >> @@ -92,16 +111,14 @@ sql_expr_type(struct Expr *pExpr) >> * return INTEGER. >> */ >> return FIELD_TYPE_INTEGER; >> - } >> - /* >> - * In case of unary plus we shouldn't discard >> - * type of operand (since plus always features >> - * NUMERIC type). >> - */ >> - if (op =3D=3D TK_UPLUS) { >> + case TK_UMINUS: >> + case TK_UPLUS: >> + case TK_NO: >> + case TK_BITNOT: >> assert(pExpr->pRight =3D=3D NULL); >> - return pExpr->pLeft->type; >> + return sql_expr_type(pExpr->pLeft); >> } >> + >=20 > 2. Stray empty line. Removed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index e1a0dfa73..4ff7169ab 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -118,7 +118,6 @@ sql_expr_type(struct Expr *pExpr) assert(pExpr->pRight =3D=3D NULL); return sql_expr_type(pExpr->pLeft); } - return pExpr->type; } >> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >> index d263c1bb5..e1a0dfa73 100644 >> --- a/src/box/sql/expr.c >> +++ b/src/box/sql/expr.c >> @@ -81,27 +57,46 @@ sqlite3ExprAffinity(Expr * pExpr) >> case TK_SELECT: >> assert(pExpr->flags & EP_xIsSelect); >> el =3D pExpr->x.pSelect->pEList; >> - return sqlite3ExprAffinity(el->a[0].pExpr); >> + return sql_expr_type(el->a[0].pExpr); >> case TK_CAST: >> assert(!ExprHasProperty(pExpr, EP_IntValue)); >> - return pExpr->affinity; >> + return pExpr->type; >> case TK_AGG_COLUMN: >> case TK_COLUMN: >> + case TK_TRIGGER: >> assert(pExpr->iColumn >=3D 0); >> - return sqlite3TableColumnAffinity(pExpr->space_def, >> - pExpr->iColumn); >> + return pExpr->space_def->fields[pExpr->iColumn].type; >> case TK_SELECT_COLUMN: >> assert(pExpr->pLeft->flags & EP_xIsSelect); >> el =3D pExpr->pLeft->x.pSelect->pEList; >> - return sqlite3ExprAffinity(el->a[pExpr->iColumn].pExpr); >> + return sql_expr_type(el->a[pExpr->iColumn].pExpr); >> case TK_PLUS: >> case TK_MINUS: >> case TK_STAR: >> case TK_SLASH: >> + case TK_REM: >> + case TK_BITAND: >> + case TK_BITOR: >> + case TK_LSHIFT: >> + case TK_RSHIFT: >> assert(pExpr->pRight !=3D NULL && pExpr->pLeft !=3D = NULL); >> - enum affinity_type lhs_aff =3D = sqlite3ExprAffinity(pExpr->pLeft); >> - enum affinity_type rhs_aff =3D = sqlite3ExprAffinity(pExpr->pRight); >> - return sql_affinity_result(rhs_aff, lhs_aff); >> + enum field_type lhs_type =3D = sql_expr_type(pExpr->pLeft); >> + enum field_type rhs_type =3D = sql_expr_type(pExpr->pRight); >> + return sql_type_result(rhs_type, lhs_type); >> + case TK_CONCAT: >> + return FIELD_TYPE_STRING; >=20 > 1. Indentation. Fixed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index e1a0dfa73..91d61b2d9 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -84,7 +84,7 @@ sql_expr_type(struct Expr *pExpr) enum field_type rhs_type =3D = sql_expr_type(pExpr->pRight); return sql_type_result(rhs_type, lhs_type); case TK_CONCAT: - return FIELD_TYPE_STRING; + return FIELD_TYPE_STRING; case TK_CASE: >> @@ -252,74 +245,57 @@ sql_expr_coll(Parse *parse, Expr *p, bool = *is_explicit_coll, uint32_t *coll_id) >> -/* >> - * pExpr is a comparison expression, eg. '=3D', '<', IN(...) etc. >> - * idx_affinity is the affinity of an indexed column. Return true >> - * if the index with affinity idx_affinity may be used to implement >> - * the comparison in pExpr. >> +/** >> + * @param expr is a comparison expression, eg. '=3D', '<', IN(...) = etc. >> + * @param field_type is the type of an indexed column. >> + * @retval Return true if the index with @field_type may be used to >> + * implement the comparison in expr. >> */ >> -int >> -sqlite3IndexAffinityOk(Expr * pExpr, char idx_affinity) >> +enum field_type >> +sql_expr_cmp_type_is_compatible(struct Expr *expr, enum field_type = field_type) >=20 > 2. Returns 'bool true', but the return value type is enum field_type? > Please, change to bool. Fixed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index e1a0dfa73..2bc4f00cf 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -285,7 +284,7 @@ expr_cmp_mutual_type(struct Expr *pExpr) * @retval Return true if the index with @field_type may be used to * implement the comparison in expr. */ -enum field_type +bool sql_expr_cmp_type_is_compatible(struct Expr *expr, enum field_type = field_type) { enum field_type type =3D expr_cmp_mutual_type(expr); switch (type) { case FIELD_TYPE_SCALAR: - return 1; + return true; case FIELD_TYPE_STRING: return field_type =3D=3D FIELD_TYPE_STRING; default: diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index d892631d8..8f75546de 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -4284,7 +4284,7 @@ sql_emit_table_types(struct Vdbe *v, struct = space_def *def, int reg); enum field_type sql_type_result(enum field_type lhs, enum field_type rhs); =20 -enum field_type +bool sql_expr_cmp_type_is_compatible(struct Expr *expr, enum field_type = idx_type); >> { >> - char aff =3D comparisonAffinity(pExpr); >> - switch (aff) { >> - case AFFINITY_BLOB: >> + enum field_type type =3D expr_cmp_mutual_type(expr); >> + switch (type) { >> + case FIELD_TYPE_SCALAR: >> return 1; >=20 > 3. 1 -> true Fixed, see above. >> @@ -2140,21 +2116,11 @@ sqlite3ExprCanBeNull(const Expr * p) >> } >> } >> -/* >> - * Return TRUE if the given expression is a constant which would be >> - * unchanged by OP_ApplyType with the type given in the second >> - * argument. >> - * >> - * This routine is used to determine if the OP_ApplyType operation >> - * can be omitted. When in doubt return FALSE. A false negative >> - * is harmless. A false positive, however, can result in the wrong >> - * answer. >> - */ >> int >> -sqlite3ExprNeedsNoAffinityChange(const Expr * p, char aff) >> +sql_expr_needs_no_type_change(const struct Expr *p, enum field_type = type) >=20 > 4. The same. Anyway you changed the function name, so without > increasing the diff you can change its return value to bool, > and the values below 0 -> false, 1 -> true. That=E2=80=99s true. Fixed: @@ -2116,12 +2115,12 @@ sqlite3ExprCanBeNull(const Expr * p) } } =20 -int +bool sql_expr_needs_no_type_change(const struct Expr *p, enum field_type = type) { u8 op; if (type =3D=3D FIELD_TYPE_SCALAR) - return 1; + return true; while (p->op =3D=3D TK_UPLUS || p->op =3D=3D TK_UMINUS) { p =3D p->pLeft; } @@ -2136,13 +2135,13 @@ sql_expr_needs_no_type_change(const struct Expr = *p, enum field_type type) case TK_STRING: return type =3D=3D FIELD_TYPE_STRING; case TK_BLOB: - return 1; + return true; case TK_COLUMN: /* p cannot be part of a CHECK constraint. */ assert(p->iTable >=3D 0); return p->iColumn < 0 && sql_type_is_numeric(type); default: - return 0; + return false; } } index d892631d8..f200752ce 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -3734,7 +3734,7 @@ int sqlite3ExprCanBeNull(const Expr *); * is harmless. A false positive, however, can result in the wrong * answer. */ -int +bool sql_expr_needs_no_type_change(const struct Expr *expr, enum field_type = type);