[tarantool-patches] Re: [PATCH 7/8] sql: clean-up affinity from SQL source code

n.pettik korablev at tarantool.org
Mon Jan 28 19:40:05 MSK 2019


>> Still some comments and variables contain ‘affinity’ term.
>> We can either accept it as a synonym to type, or fix
>> it within only code-style-fix commit.
> 
> I think, we should rename. As soon as possible. Otherwise we
> will have 'affinity' in our code base for ages. Together with
> comments, dead code, already changed in this patch code, and
> the code you remove in the next commit my text editor found
> 199 'affinity' usages. It is not too much. Also, I am almost
> sure, that you will find some bugs during the renaming process.

Ok, I’ve dealt with it. You may say that some renaming/refactoring
rather belongs to other patches, but lets consider this patch as a
follow-up containing code-style fixes. It seems to be quite complicated
to split these fixes to proper parts.

diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index a6e7547f5..5a3be4862 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -587,7 +587,7 @@ sql_generate_index_key(struct Parse *parse, struct index *index, int cursor,
                sqlite3ExprCodeGetColumnOfTable(v, space->def, cursor, tabl_col,
                                                reg_base + j);
                /*
-                * If the column affinity is REAL but the number
+                * If the column type is NUMBER but the number
                 * is an integer, then it might be stored in the
                 * table as an integer (using a compact
                 * representation) then converted to REAL by an
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ffb3f8bb8..c3fd70427 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -267,7 +267,7 @@ sql_type_result(enum field_type lhs, enum field_type rhs)
 }
 
 /**
- * pExpr is a comparison operator. Return the type affinity
+ * pExpr is a comparison operator. Return the type
  * that should be applied to both operands prior to doing
  * the comparison.
  */
@@ -2391,15 +2391,15 @@ sqlite3FindInIndex(Parse * pParse,      /* Parsing context */
                pTab = p->pSrc->a[0].pTab;
                assert(v);      /* sqlite3GetVdbe() has always been previously called */
 
-               int affinity_ok = 1;
+               bool type_is_suitable = true;
                int i;
 
-               /* Check that the affinity that will be used to perform each
-                * comparison is the same as the affinity of each column in table
+               /* Check that the type that will be used to perform each
+                * comparison is the same as the type of each column in table
                 * on the RHS of the IN operator.  If it not, it is not possible to
                 * use any index of the RHS table.
                 */
-               for (i = 0; i < nExpr && affinity_ok; i++) {
+               for (i = 0; i < nExpr && type_is_suitable; i++) {
                        Expr *pLhs = sqlite3VectorFieldSubexpr(pX->pLeft, i);
                        int iCol = pEList->a[i].pExpr->iColumn;
                        /* RHS table */
@@ -2411,10 +2411,10 @@ sqlite3FindInIndex(Parse * pParse,      /* Parsing context */
                         * of columns match.
                         */
                        if (idx_type != lhs_type)
-                               affinity_ok = 0;
+                               type_is_suitable = false;
                }
 
-               if (affinity_ok) {
+               if (type_is_suitable) {
                        /*
                         * Here we need real space since further
                         * it is used in cursor opening routine.
@@ -2531,7 +2531,7 @@ sqlite3FindInIndex(Parse * pParse,        /* Parsing context */
                                        sqlite3VdbeJumpHere(v, iAddr);
                                }
                        }       /* End loop over indexes */
-               }       /* End if( affinity_ok ) */
+               }
        }
 
        /* End attempt to optimize using an index */
@@ -2580,14 +2580,14 @@ sqlite3FindInIndex(Parse * pParse,      /* Parsing context */
 
 /*
  * Argument pExpr is an (?, ?...) IN(...) expression. This
- * function allocates and returns a nul-terminated string containing
- * the affinities to be used for each column of the comparison.
+ * function allocates and returns a terminated string containing
+ * the types to be used for each column of the comparison.
  *
  * It is the responsibility of the caller to ensure that the returned
  * string is eventually freed using sqlite3DbFree().
  */
 static enum field_type *
-exprINAffinity(Parse * pParse, Expr * pExpr)
+expr_in_type(Parse *pParse, Expr *pExpr)
 {
        Expr *pLeft = pExpr->pLeft;
        int nVal = sqlite3ExprVectorSize(pLeft);
@@ -2606,8 +2606,6 @@ exprINAffinity(Parse * pParse, Expr * pExpr)
                                enum field_type rhs = sql_expr_type(e);
                                zRet[i] = sql_type_result(rhs, lhs);
                        } else {
-                               if (lhs == FIELD_TYPE_ANY)
-                                       lhs = FIELD_TYPE_SCALAR;
                                zRet[i] = lhs;
                        }
                }
@@ -2725,12 +2723,12 @@ sqlite3CodeSubselect(Parse * pParse,    /* Parsing context */
                         * SELECT or the <exprlist>.
                         *
                         * If the 'x' expression is a column value, or the SELECT...
-                        * statement returns a column value, then the affinity of that
+                        * statement returns a column value, then the type of that
                         * column is used to build the index keys. If both 'x' and the
-                        * SELECT... statement are columns, then numeric affinity is used
-                        * if either column has NUMERIC or INTEGER affinity. If neither
-                        * 'x' nor the SELECT... statement are columns, then numeric affinity
-                        * is used.
+                        * SELECT... statement are columns, then NUMBER type is used
+                        * if either column has NUMBER or INTEGER type. If neither
+                        * 'x' nor the SELECT... statement are columns,
+                        * then NUMBER type is used.
                         */
                        pExpr->iTable = pParse->nTab++;
                        pExpr->is_ephemeral = 1;
@@ -2760,8 +2758,8 @@ sqlite3CodeSubselect(Parse * pParse,      /* Parsing context */
                                        int i;
                                        sqlite3SelectDestInit(&dest, SRT_Set,
                                                              pExpr->iTable, reg_eph);
-                                       dest.zAffSdst =
-                                           exprINAffinity(pParse, pExpr);
+                                       dest.dest_type =
+                                               expr_in_type(pParse, pExpr);
                                        assert((pExpr->iTable & 0x0000FFFF) ==
                                               pExpr->iTable);
                                        pSelect->iLimit = 0;
@@ -2770,12 +2768,12 @@ sqlite3CodeSubselect(Parse * pParse,    /* Parsing context */
                                        if (sqlite3Select
                                            (pParse, pSelect, &dest)) {
                                                sqlite3DbFree(pParse->db,
-                                                             dest.zAffSdst);
+                                                             dest.dest_type);
                                                sql_key_info_unref(key_info);
                                                return 0;
                                        }
                                        sqlite3DbFree(pParse->db,
-                                                     dest.zAffSdst);
+                                                     dest.dest_type);
                                        assert(pEList != 0);
                                        assert(pEList->nExpr > 0);
                                        for (i = 0; i < nVal; i++) {
@@ -2792,8 +2790,8 @@ sqlite3CodeSubselect(Parse * pParse,      /* Parsing context */
                                 *
                                 * For each expression, build an index key from the evaluation and
                                 * store it in the temporary table. If <expr> is a column, then use
-                                * that columns affinity when building index keys. If <expr> is not
-                                * a column, use numeric affinity.
+                                * that columns types when building index keys. If <expr> is not
+                                * a column, use NUMBER type.
                                 */
                                int i;
                                ExprList *pList = pExpr->x.pList;
@@ -2999,7 +2997,7 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */
        if (sqlite3ExprCheckIN(pParse, pExpr))
                return;
        /* Type sequence for comparisons. */
-       enum field_type *zAff = exprINAffinity(pParse, pExpr);
+       enum field_type *zAff = expr_in_type(pParse, pExpr);
        nVector = sqlite3ExprVectorSize(pExpr->pLeft);
        aiMap =
            (int *)sqlite3DbMallocZero(pParse->db,
@@ -3981,7 +3979,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
                        } else {
                                /*
                                 * Otherwise, use first arg as
-                                * expression affinity.
+                                * expression type.
                                 */
                                if (pFarg && pFarg->nExpr > 0)
                                        pExpr->type = pFarg->a[0].pExpr->type;
@@ -4209,11 +4207,8 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
                                        pExpr->iColumn].name, target));
 
 #ifndef SQLITE_OMIT_FLOATING_POINT
-                       /* If the column has REAL affinity, it may currently be stored as an
+                       /* If the column has type NUMBER, it may currently be stored as an
                         * integer. Use OP_Realify to make sure it is really real.
-                        *
-                        * EVIDENCE-OF: R-60985-57662 SQLite will convert the value back to
-                        * floating point when extracting it from the record.
                         */
                        if (pExpr->iColumn >= 0 && def->fields[
                                pExpr->iColumn].type == FIELD_TYPE_NUMBER) {
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index d5a728382..01fe1c84d 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -423,7 +423,7 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src, struct Table *tab,
         * <parent-key1> = <child-key1> AND <parent-key2> = <child-key2> ...
         *
         * The collation sequence used for the comparison should
-        * be that of the parent key columns. The affinity of the
+        * be that of the parent key columns. The type of the
         * parent key column should be applied to each child key
         * value before the comparison takes place.
         */
@@ -783,7 +783,7 @@ fkey_action_trigger(struct Parse *pParse, struct Table *pTab, struct fkey *fkey,
                 * Create the expression "old.to_col = from_col".
                 * It is important that the "old.to_col" term is
                 * on the LHS of the = operator, so that the
-                * affinity and collation sequence associated with
+                * type and collation sequence associated with
                 * the parent table are used for the comparison.
                 */
                struct Expr *to_col =
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index f4e5b1ec0..c69476117 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -603,7 +603,7 @@ sqlite3Insert(Parse * pParse,       /* Parser context */
                /* If this is an INSERT on a view with an INSTEAD OF INSERT trigger,
                 * do not attempt any conversions before assembling the record.
                 * If this is a real table, attempt conversions as required by the
-                * table column affinities.
+                * table column types.
                 */
                if (!is_view)
                        sql_emit_table_types(v, pTab->def, regCols + 1);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 774a34fd7..336c7af7a 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -738,7 +738,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
                                }
                                pNC->ncFlags |= NC_AllowAgg;
                        }
-                       /* FIX ME:  Compute pExpr->affinity based on the expected return
+                       /* FIX ME:  Compute pExpr->type based on the expected return
                         * type of the function
                         */
                        return WRC_Prune;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index d582a1f19..714c0c34a 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -134,7 +134,7 @@ sqlite3SelectDestInit(SelectDest * pDest, int eDest, int iParm, int reg_eph)
        pDest->eDest = (u8) eDest;
        pDest->iSDParm = iParm;
        pDest->reg_eph = reg_eph;
-       pDest->zAffSdst = 0;
+       pDest->dest_type = NULL;
        pDest->iSdst = 0;
        pDest->nSdst = 0;
 }
@@ -1202,7 +1202,7 @@ selectInnerLoop(Parse * pParse,           /* The parser context */
                                int r1 = sqlite3GetTempReg(pParse);
                                enum field_type *types =
                                        field_type_sequence_dup(pParse,
-                                                               pDest->zAffSdst,
+                                                               pDest->dest_type,
                                                                nResultCol);
                                sqlite3VdbeAddOp4(v, OP_MakeRecord, regResult,
                                                  nResultCol, r1, (char *)types,
@@ -1626,7 +1626,7 @@ generateSortTail(Parse * pParse,  /* Parsing context */
                }
        case SRT_Set:{
                        enum field_type *types =
-                               field_type_sequence_dup(pParse, pDest->zAffSdst,
+                               field_type_sequence_dup(pParse, pDest->dest_type,
                                                        nColumn);
                        sqlite3VdbeAddOp4(v, OP_MakeRecord, regRow, nColumn,
                                          regTupleid, (char *)types,
@@ -3055,7 +3055,7 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,
                        testcase(in->nSdst > 1);
                        r1 = sqlite3GetTempReg(parse);
                        enum field_type *types =
-                               field_type_sequence_dup(parse, dest->zAffSdst,
+                               field_type_sequence_dup(parse, dest->dest_type,
                                                        in->nSdst);
                        sqlite3VdbeAddOp4(v, OP_MakeRecord, in->iSdst,
                                          in->nSdst, r1, (char *)types,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 260431d6d..88813b674 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1629,7 +1629,7 @@ struct sqlite3 {
 #define SQLITE_MAGIC_ZOMBIE   0x64cffc7f       /* Close with last statement close */
 
 /**
- * SQL type definition. Now it is an alias to affinity, but in
+ * SQL type definition. Now it is an alias to type, but in
  * future it will have some attributes like number of chars in
  * VARCHAR(<number of chars>).
  */
@@ -1793,8 +1793,8 @@ struct Savepoint {
                                 (X) == FIELD_TYPE_UNSIGNED)
 
 /*
- * Additional bit values that can be ORed with an affinity without
- * changing the affinity.
+ * Additional bit values that can be ORed with an type without
+ * changing the type.
  *
  * The SQLITE_NOTNULL flag is a combination of NULLEQ and JUMPIFNULL.
  * It causes an assert() to fire if either operand to a comparison
@@ -2550,7 +2550,7 @@ struct Select {
  *
  *     SRT_Set         The result must be a single column.  Store each
  *                     row of result as the key in table pDest->iSDParm.
- *                     Apply the affinity pDest->affSdst before storing
+ *                     Apply the type pDest->det_type before storing
  *                     results.  Used to implement "IN (SELECT ...)".
  *
  *     SRT_EphemTab    Create an temporary table pDest->iSDParm and store
@@ -2610,7 +2610,7 @@ struct Select {
 struct SelectDest {
        u8 eDest;               /* How to dispose of the results.  On of SRT_* above. */
        /** Type used when eDest==SRT_Set */
-       enum field_type *zAffSdst;
+       enum field_type *dest_type;
        int iSDParm;            /* A parameter used by the eDest disposal method */
        /** Register containing ephemeral's space pointer. */
        int reg_eph;
@@ -4371,7 +4371,7 @@ void sqlite3ValueFree(sqlite3_value *);
 sqlite3_value *sqlite3ValueNew(sqlite3 *);
 int sqlite3ValueFromExpr(sqlite3 *, Expr *, enum field_type type,
                         sqlite3_value **);
-void sqlite3ValueApplyAffinity(sqlite3_value *, enum field_type type);
+void sql_value_apply_type(sqlite3_value *val, enum field_type type);
 #ifndef SQLITE_AMALGAMATION
 extern const unsigned char sqlite3OpcodeProperty[];
 extern const char sqlite3StrBINARY[];
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index df59f6a0a..c98fcfabd 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -289,7 +289,7 @@ allocateCursor(
  * if there is an exact integer representation of the quantity.
  */
 static int
-applyNumericAffinity(Mem *pRec, int bTryForInt)
+mem_apply_numeric_type(Mem *pRec, int bTryForInt)
 {
        double rValue;
        i64 iValue;
@@ -301,7 +301,7 @@ applyNumericAffinity(Mem *pRec, int bTryForInt)
        } else {
                pRec->u.r = rValue;
                pRec->flags |= MEM_Real;
-               if (bTryForInt) sqlite3VdbeIntegerAffinity(pRec);
+               if (bTryForInt) mem_apply_integer_type(pRec);
        }
        return 0;
 }
@@ -382,7 +382,7 @@ int sqlite3_value_numeric_type(sqlite3_value *pVal) {
        int eType = sqlite3_value_type(pVal);
        if (eType==SQLITE_TEXT) {
                Mem *pMem = (Mem*)pVal;
-               applyNumericAffinity(pMem, 0);
+               mem_apply_numeric_type(pMem, 0);
                eType = sqlite3_value_type(pVal);
        }
        return eType;
@@ -393,7 +393,7 @@ int sqlite3_value_numeric_type(sqlite3_value *pVal) {
  * not the internal Mem* type.
  */
 void
-sqlite3ValueApplyAffinity(
+sql_value_apply_type(
        sqlite3_value *pVal,
        enum field_type type)
 {
@@ -421,7 +421,7 @@ static u16 SQLITE_NOINLINE computeNumericType(Mem *pMem)
  * Return the numeric type for pMem, either MEM_Int or MEM_Real or both or
  * none.
  *
- * Unlike applyNumericAffinity(), this routine does not modify pMem->flags.
+ * Unlike mem_apply_numeric_type(), this routine does not modify pMem->flags.
  * But it does set pMem->u.r and pMem->u.i appropriately.
  */
 static u16 numericType(Mem *pMem)
@@ -1681,7 +1681,7 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
                pOut->u.r = rB;
                MemSetTypeFlag(pOut, MEM_Real);
                if (((type1|type2)&MEM_Real)==0 && !bIntint) {
-                       sqlite3VdbeIntegerAffinity(pOut);
+                       mem_apply_integer_type(pOut);
                }
 #endif
        }
@@ -2107,7 +2107,6 @@ case OP_Le:               /* same as TK_LE, jump, in1, in3 */
 case OP_Gt:               /* same as TK_GT, jump, in1, in3 */
 case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
        int res, res2;      /* Result of the comparison of pIn1 against pIn3 */
-       char affinity;      /* Affinity to use for comparison */
        u32 flags1;         /* Copy of initial value of pIn1->flags */
        u32 flags3;         /* Copy of initial value of pIn3->flags */
 
@@ -2160,16 +2159,16 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
                 * p5 with this constant. Node that all other flags
                 * that can be stored in p5 are >= 16.
                 */
-               affinity = pOp->p5 & 15;
-               if (sql_type_is_numeric(affinity)) {
+               enum field_type type = pOp->p5 & 15;
+               if (sql_type_is_numeric(type)) {
                        if ((flags1 | flags3)&MEM_Str) {
                                if ((flags1 & (MEM_Int|MEM_Real|MEM_Str))==MEM_Str) {
-                                       applyNumericAffinity(pIn1,0);
+                                       mem_apply_numeric_type(pIn1, 0);
                                        testcase( flags3!=pIn3->flags); /* Possible if pIn1==pIn3 */
                                        flags3 = pIn3->flags;
                                }
                                if ((flags3 & (MEM_Int|MEM_Real|MEM_Str))==MEM_Str) {
-                                       if (applyNumericAffinity(pIn3,0) != 0) {
+                                       if (mem_apply_numeric_type(pIn3, 0) != 0) {
                                                diag_set(ClientError,
                                                         ER_SQL_TYPE_MISMATCH,
                                                         sqlite3_value_text(pIn3),
@@ -2189,7 +2188,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
                                res = 0;
                                goto compare_op;
                        }
-               } else if (affinity == FIELD_TYPE_STRING) {
+               } else if (type == FIELD_TYPE_STRING) {
                        if ((flags1 & MEM_Str)==0 && (flags1 & (MEM_Int|MEM_Real))!=0) {
                                testcase( pIn1->flags & MEM_Int);
                                testcase( pIn1->flags & MEM_Real);
@@ -2837,7 +2836,7 @@ case OP_ApplyType: {
  * in an index.  The OP_Column opcode can decode the record later.
  *
  * P4 may be a string that is P2 characters long.  The nth character of the
- * string indicates the column affinity that should be used for the nth
+ * string indicates the column type that should be used for the nth
  * field of the index key.
  *
  * If P4 is NULL then all index fields have type SCALAR.
@@ -2880,8 +2879,7 @@ case OP_MakeRecord: {
        pOut = &aMem[pOp->p3];
        memAboutToChange(p, pOut);
 
-       /* Apply the requested affinity to all inputs
-        */
+       /* Apply the requested types to all inputs */
        assert(pData0<=pLast);
        if (types != NULL) {
                pRec = pData0;
@@ -3513,7 +3511,7 @@ case OP_SeekGT: {       /* jump, in3 */
                 */
                pIn3 = &aMem[reg_ipk];
                if ((pIn3->flags & (MEM_Int|MEM_Real|MEM_Str))==MEM_Str) {
-                       applyNumericAffinity(pIn3, 0);
+                       mem_apply_numeric_type(pIn3, 0);
                }
                int64_t i;
                if ((pIn3->flags & MEM_Int) == MEM_Int) {
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 32b1b6c17..1720fd1c7 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -231,7 +231,6 @@ struct Mem {
 #define MEM_Blob      0x0010   /* Value is a BLOB */
 #define MEM_Bool      0x0020    /* Value is a bool */
 #define MEM_Ptr       0x0040   /* Value is a generic pointer */
-#define MEM_AffMask   0x003f   /* Mask of affinity bits */
 #define MEM_Frame     0x0080   /* Value is a VdbeFrame object */
 #define MEM_Undefined 0x0100   /* Value is undefined */
 #define MEM_Cleared   0x0200   /* NULL set by OP_Null, not from data */
@@ -480,7 +479,7 @@ int sqlite3VdbeMemStringify(Mem *, u8);
 int sqlite3VdbeIntValue(Mem *, int64_t *);
 int sqlite3VdbeMemIntegerify(Mem *, bool is_forced);
 int sqlite3VdbeRealValue(Mem *, double *);
-int sqlite3VdbeIntegerAffinity(Mem *);
+int mem_apply_integer_type(Mem *);
 int sqlite3VdbeMemRealify(Mem *);
 int sqlite3VdbeMemNumerify(Mem *);
 int sqlite3VdbeMemCast(Mem *, enum field_type type);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 9e57af051..07c496207 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -280,7 +280,7 @@ sqlite3_value_type(sqlite3_value * pVal)
                SQLITE_INTEGER, /* 0x1e */
                SQLITE_NULL,    /* 0x1f */
        };
-       return aType[pVal->flags & MEM_AffMask];
+       return aType[pVal->flags & MEM_TypeMask];
 }
 
 /* Make a copy of an sqlite3_value object
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index b37a7141f..5f2cb8448 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -3507,7 +3507,7 @@ sqlite3VdbeGetBoundValue(Vdbe * v, int iVar, u8 aff)
                        sqlite3_value *pRet = sqlite3ValueNew(v->db);
                        if (pRet) {
                                sqlite3VdbeMemCopy((Mem *) pRet, pMem);
-                               sqlite3ValueApplyAffinity(pRet, aff);
+                               sql_value_apply_type(pRet, aff);
                        }
                        return pRet;
                }
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index a5dd7400a..2865fd68c 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -504,7 +504,7 @@ sqlite3VdbeRealValue(Mem * pMem, double *v)
  * MEM_Int if we can.
  */
 int
-sqlite3VdbeIntegerAffinity(Mem * pMem)
+mem_apply_integer_type(Mem *pMem)
 {
        int rc;
        i64 ix;
@@ -584,7 +584,7 @@ sqlite3VdbeMemNumerify(Mem * pMem)
                                return SQLITE_ERROR;
                        pMem->u.r = v;
                        MemSetTypeFlag(pMem, MEM_Real);
-                       sqlite3VdbeIntegerAffinity(pMem);
+                       mem_apply_integer_type(pMem);
                }
        }
        assert((pMem->flags & (MEM_Int | MEM_Real | MEM_Null)) != 0);
@@ -593,10 +593,10 @@ sqlite3VdbeMemNumerify(Mem * pMem)
 }
 
 /*
- * Cast the datatype of the value in pMem according to the affinity
- * "aff".  Casting is different from applying affinity in that a cast
+ * Cast the datatype of the value in pMem according to the type
+ * @type.  Casting is different from applying type in that a cast
  * is forced.  In other words, the value is converted into the desired
- * affinity even if that results in loss of data.  This routine is
+ * type even if that results in loss of data.  This routine is
  * used (for example) to implement the SQL "cast()" operator.
  */
 int
@@ -643,7 +643,7 @@ sqlite3VdbeMemCast(Mem * pMem, enum field_type type)
                assert(type == FIELD_TYPE_STRING);
                assert(MEM_Str == (MEM_Blob >> 3));
                pMem->flags |= (pMem->flags & MEM_Blob) >> 3;
-               sqlite3ValueApplyAffinity(pMem, FIELD_TYPE_STRING);
+                       sql_value_apply_type(pMem, FIELD_TYPE_STRING);
                assert(pMem->flags & MEM_Str || pMem->db->mallocFailed);
                pMem->flags &= ~(MEM_Int | MEM_Real | MEM_Blob | MEM_Zero);
                return SQLITE_OK;
@@ -1152,7 +1152,7 @@ valueNew(sqlite3 * db, struct ValueNewStat4Ctx *p)
  * error occurs, output parameter (*ppVal) is set to point to a value
  * object containing the result before returning SQLITE_OK.
  *
- * Affinity aff is applied to the result of the function before returning.
+ * Type @type is applied to the result of the function before returning.
  * If the result is a text value, the sqlite3_value object uses encoding
  * enc.
  *
@@ -1222,7 +1222,7 @@ valueFromFunction(sqlite3 * db,   /* The database connection */
                rc = ctx.isError;
                sqlite3ErrorMsg(pCtx->pParse, "%s", sqlite3_value_text(pVal));
        } else {
-               sqlite3ValueApplyAffinity(pVal, type);
+               sql_value_apply_type(pVal, type);
                assert(rc == SQLITE_OK);
        }
        pCtx->pParse->rc = rc;
@@ -1285,7 +1285,7 @@ valueFromExpr(sqlite3 * db,       /* The database connection */
                testcase(rc != SQLITE_OK);
                if (*ppVal) {
                        sqlite3VdbeMemCast(*ppVal, pExpr->type);
-                       sqlite3ValueApplyAffinity(*ppVal, type);
+                       sql_value_apply_type(*ppVal, type);
                }
                return rc;
        }
@@ -1318,9 +1318,9 @@ valueFromExpr(sqlite3 * db,       /* The database connection */
                }
                if ((op == TK_INTEGER || op == TK_FLOAT) &&
                    type == FIELD_TYPE_SCALAR) {
-                       sqlite3ValueApplyAffinity(pVal, FIELD_TYPE_NUMBER);
+                       sql_value_apply_type(pVal, FIELD_TYPE_NUMBER);
                } else {
-                       sqlite3ValueApplyAffinity(pVal, type);
+                       sql_value_apply_type(pVal, type);
                }
                if (pVal->flags & (MEM_Int | MEM_Real))
                        pVal->flags &= ~MEM_Str;
@@ -1339,7 +1339,7 @@ valueFromExpr(sqlite3 * db,       /* The database connection */
                        } else {
                                pVal->u.i = -pVal->u.i;
                        }
-                       sqlite3ValueApplyAffinity(pVal, type);
+                       sql_value_apply_type(pVal, type);
                }
        } else if (op == TK_NULL) {
                pVal = valueNew(db, pCtx);
@@ -1502,7 +1502,7 @@ stat4ValueFromExpr(Parse * pParse,        /* Parse context */
                                rc = sqlite3VdbeMemCopy((Mem *) pVal,
                                                        &v->aVar[iBindVar - 1]);
                                if (rc == SQLITE_OK) {
-                                       sqlite3ValueApplyAffinity(pVal, type);
+                                       sql_value_apply_type(pVal, type);
                                }
                                pVal->db = pParse->db;
                        }
@@ -1536,7 +1536,7 @@ stat4ValueFromExpr(Parse * pParse,        /* Parse context */
  * vector components that match either of the two latter criteria listed
  * above.
  *
- * Before any value is appended to the record, the affinity of the
+ * Before any value is appended to the record, the type of the
  * corresponding column within index pIdx is applied to it. Before
  * this function returns, output parameter *pnExtract is set to the
  * number of values appended to the record.
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 6b3383bec..889a3517b 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -295,7 +295,7 @@ whereScanNext(WhereScan * pScan)
                                        }
                                        if ((pTerm->eOperator & pScan->
                                             opMask) != 0) {
-                                               /* Verify the affinity and collating sequence match */
+                                               /* Verify the type and collating sequence match */
                                                if ((pTerm->eOperator & WO_ISNULL) == 0) {
                                                        pX = pTerm->pExpr;
                                                        if (!sql_expr_cmp_type_is_compatible(pX, pScan->idx_type))
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index ca375844f..7d657ad53 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -406,28 +406,25 @@ emit_apply_type(Parse *pParse, int base, int n, enum field_type *types)
        }
 }
 
-/*
- * Expression pRight, which is the RHS of a comparison operation, is
+/**
+ * Expression @rhs, which is the RHS of a comparison operation, is
  * either a vector of n elements or, if n==1, a scalar expression.
- * Before the comparison operation, affinity zAff is to be applied
- * to the pRight values. This function modifies entries within the
+ * Before the comparison operation, types @types are to be applied
+ * to the @rhs values. This function modifies entries within the
  * field sequence to SCALAR if either:
  *
- *   * the comparison will be performed with no affinity, or
- *   * the affinity change in zAff is guaranteed not to change the value.
+ *   * the comparison will be performed with no type, or
+ *   * the type change in @types is guaranteed not to change the value.
  */
 static void
-updateRangeAffinityStr(Expr * pRight,  /* RHS of comparison */
-                      int n,           /* Number of vector elements in comparison */
-                      enum field_type *zAff)   /* Affinity string to modify */
+expr_cmp_update_rhs_type(struct Expr *rhs, int n, enum field_type *types)
 {
-       int i;
-       for (i = 0; i < n; i++) {
-               Expr *p = sqlite3VectorFieldSubexpr(pRight, i);
+       for (int i = 0; i < n; i++) {
+               Expr *p = sqlite3VectorFieldSubexpr(rhs, i);
                enum field_type expr_type = sql_expr_type(p);
-               if (sql_type_result(expr_type, zAff[i]) == FIELD_TYPE_SCALAR ||
-                   sql_expr_needs_no_type_change(p, zAff[i])) {
-                       zAff[i] = FIELD_TYPE_SCALAR;
+               if (sql_type_result(expr_type, types[i]) == FIELD_TYPE_SCALAR ||
+                   sql_expr_needs_no_type_change(p, types[i])) {
+                       types[i] = FIELD_TYPE_SCALAR;
                }
        }
 }
@@ -651,7 +648,7 @@ codeEqualityTerm(Parse * pParse,    /* The parsing context */
  * In the example above nEq==2.  But this subroutine works for any value
  * of nEq including 0.  If nEq==0, this routine is nearly a no-op.
  * The only thing it does is allocate the pLevel->iMem memory cell and
- * compute the affinity string.
+ * compute the types array.
  *
  * The nExtraReg parameter is 0 or 1.  It is 0 if all WHERE clause constraints
  * are == or IN and are covered by the nEq.  nExtraReg is 1 if there is
@@ -666,19 +663,19 @@ codeEqualityTerm(Parse * pParse,  /* The parsing context */
  * this routine allocates an additional nEq memory cells for internal
  * use.
  *
- * Before returning, *pzAff is set to point to a buffer containing a
- * copy of the column affinity string of the index allocated using
+ * Before returning, @types is set to point to a buffer containing a
+ * copy of the column types array of the index allocated using
  * sqlite3DbMalloc(). Except, entries in the copy of the string associated
- * with equality constraints that use BLOB or NONE affinity are set to
+ * with equality constraints that use SCALAR type are set to
  * SCALAR. This is to deal with SQL such as the following:
  *
- *   CREATE TABLE t1(a TEXT PRIMARY KEY, b);
+ *   CREATE TABLE t1(a TEXT PRIMARY KEY, b BLOB);
  *   SELECT ... FROM t1 AS t2, t1 WHERE t1.a = t2.b;
  *
- * In the example above, the index on t1(a) has TEXT affinity. But since
- * the right hand side of the equality constraint (t2.b) has BLOB/NONE affinity,
+ * In the example above, the index on t1(a) has STRING type. But since
+ * the right hand side of the equality constraint (t2.b) has SCALAR type,
  * no conversion should be attempted before using a t2.b value as part of
- * a key to search the index. Hence the first byte in the returned affinity
+ * a key to search the index. Hence the first byte in the returned type
  * string in this example would be set to SCALAR.
  */
 static int
@@ -686,7 +683,7 @@ codeAllEqualityTerms(Parse * pParse,        /* Parsing context */
                     WhereLevel * pLevel,       /* Which nested loop of the FROM we are coding */
                     int bRev,          /* Reverse the order of IN operators */
                     int nExtraReg,     /* Number of extra registers to allocate */
-                    enum field_type **pzAff)   /* OUT: Set to point to affinity string */
+                    enum field_type **res_type)
 {
        u16 nEq;                /* The number of == or IN constraints to code */
        u16 nSkip;              /* Number of left-most columns to skip */
@@ -710,8 +707,8 @@ codeAllEqualityTerms(Parse * pParse,        /* Parsing context */
        nReg = pLoop->nEq + nExtraReg;
        pParse->nMem += nReg;
 
-       enum field_type *zAff = sql_index_type_str(pParse->db, idx_def);
-       assert(zAff != 0 || pParse->db->mallocFailed);
+       enum field_type *type = sql_index_type_str(pParse->db, idx_def);
+       assert(type != NULL || pParse->db->mallocFailed);
 
        if (nSkip) {
                int iIdxCur = pLevel->iIdxCur;
@@ -757,13 +754,13 @@ codeAllEqualityTerms(Parse * pParse,      /* Parsing context */
                }
                if (pTerm->eOperator & WO_IN) {
                        if (pTerm->pExpr->flags & EP_xIsSelect) {
-                               /* No affinity ever needs to be (or should be) applied to a value
+                               /* No type ever needs to be (or should be) applied to a value
                                 * from the RHS of an "? IN (SELECT ...)" expression. The
                                 * sqlite3FindInIndex() routine has already ensured that the
-                                * affinity of the comparison has been applied to the value.
+                                * type of the comparison has been applied to the value.
                                 */
-                               if (zAff)
-                                       zAff[j] = FIELD_TYPE_SCALAR;
+                               if (type != NULL)
+                                       type[j] = FIELD_TYPE_SCALAR;
                        }
                } else if ((pTerm->eOperator & WO_ISNULL) == 0) {
                        Expr *pRight = pTerm->pExpr->pRight;
@@ -772,20 +769,19 @@ codeAllEqualityTerms(Parse * pParse,      /* Parsing context */
                                                  pLevel->addrBrk);
                                VdbeCoverage(v);
                        }
-                       if (zAff) {
-                               enum field_type type =
+                       if (type != NULL) {
+                               enum field_type rhs_type =
                                        sql_expr_type(pRight);
-                               enum field_type idx_type = zAff[j];
-                               if (sql_type_result(type, idx_type) ==
+                               if (sql_type_result(rhs_type, type[j]) ==
                                    FIELD_TYPE_SCALAR) {
-                                       zAff[j] = FIELD_TYPE_SCALAR;
+                                       type[j] = FIELD_TYPE_SCALAR;
                                }
-                               if (sql_expr_needs_no_type_change(pRight, idx_type))
-                                       zAff[j] = FIELD_TYPE_SCALAR;
+                               if (sql_expr_needs_no_type_change(pRight, type[j]))
+                                       type[j] = FIELD_TYPE_SCALAR;
                        }
                }
        }
-       *pzAff = zAff;
+       *res_type = type;
        return regBase;
 }
 
@@ -994,8 +990,10 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,   /* Complete information about t
                int iIdxCur;    /* The VDBE cursor for the index */
                int nExtraReg = 0;      /* Number of extra registers needed */
                int op;         /* Instruction opcode */
-               enum field_type *zStartAff;     /* Affinity for start of range constraint */
-               enum field_type *zEndAff = NULL;        /* Affinity for end of range constraint */
+               /* Types for start of range constraint. */
+               enum field_type *start_types;
+               /* Types for end of range constraint */
+               enum field_type *end_types = NULL;
                u8 bSeekPastNull = 0;   /* True to seek past initial nulls */
                u8 bStopAtNull = 0;     /* Add condition to terminate at NULLs */
                int force_integer_reg = -1;  /* If non-negative: number of
@@ -1109,14 +1107,14 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,        /* Complete information about t
                 */
                regBase =
                    codeAllEqualityTerms(pParse, pLevel, bRev, nExtraReg,
-                                        &zStartAff);
-               if (zStartAff != NULL && nTop) {
+                                        &start_types);
+               if (start_types != NULL && nTop) {
                        uint32_t len = 0;
-                       for (enum field_type *tmp = &zStartAff[nEq];
+                       for (enum field_type *tmp = &start_types[nEq];
                             *tmp != field_type_MAX; tmp++, len++);
                        uint32_t sz = len * sizeof(enum field_type);
-                       zEndAff = sqlite3DbMallocRaw(db, sz);
-                       memcpy(zEndAff, &zStartAff[nEq], sz);
+                       end_types = sqlite3DbMallocRaw(db, sz);
+                       memcpy(end_types, &start_types[nEq], sz);
                }
                addrNxt = pLevel->addrNxt;
 
@@ -1144,9 +1142,9 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,  /* Complete information about t
                                VdbeCoverage(v);
                        }
 
-                       if (zStartAff) {
-                               updateRangeAffinityStr(pRight, nBtm,
-                                                      &zStartAff[nEq]);
+                       if (start_types) {
+                               expr_cmp_update_rhs_type(pRight, nBtm,
+                                                        &start_types[nEq]);
                        }
                        nConstraint += nBtm;
                        testcase(pRangeStart->wtFlags & TERM_VIRTUAL);
@@ -1191,7 +1189,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,  /* Complete information about t
                        }
                }
                emit_apply_type(pParse, regBase, nConstraint - bSeekPastNull,
-                               zStartAff);
+                               start_types);
                if (pLoop->nSkip > 0 && nConstraint == pLoop->nSkip) {
                        /* The skip-scan logic inside the call to codeAllEqualityConstraints()
                         * above has already left the cursor sitting on the correct row,
@@ -1241,10 +1239,10 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,        /* Complete information about t
                                                  addrNxt);
                                VdbeCoverage(v);
                        }
-                       if (zEndAff) {
-                               updateRangeAffinityStr(pRight, nTop, zEndAff);
+                       if (end_types) {
+                               expr_cmp_update_rhs_type(pRight, nTop, end_types);
                                emit_apply_type(pParse, regBase + nEq, nTop,
-                                               zEndAff);
+                                               end_types);
                        } else {
                                assert(pParse->db->mallocFailed);
                        }
@@ -1261,8 +1259,8 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,  /* Complete information about t
                        endEq = 0;
                        nConstraint++;
                }
-               sqlite3DbFree(db, zStartAff);
-               sqlite3DbFree(db, zEndAff);
+               sqlite3DbFree(db, start_types);
+               sqlite3DbFree(db, end_types);
 
                /* Top of the loop body */
                pLevel->p2 = sqlite3VdbeCurrentAddr(v);
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index a38ffed48..b016d18ef 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -272,7 +272,7 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix,
            sql_expr_type(pLeft) != FIELD_TYPE_STRING) {
                /* IMP: R-02065-49465 The left-hand side of the
                 * LIKE operator must be the name of an indexed
-                * column with TEXT affinity.
+                * column with STRING type.
                 */
                return 0;
        }
@@ -749,7 +749,7 @@ exprAnalyzeOrTerm(SrcList * pSrc,   /* the FROM clause */
                                } else if (pOrTerm->u.leftColumn != iColumn) {
                                        okToChngToIN = 0;
                                } else {
-                                       /* If the right-hand side is also a column, then the affinities
+                                       /* If the right-hand side is also a column, then the types
                                         * of both right and left sides must be such that no type
                                         * conversions are required on the right.  (Ticket #2249)
                                         */
@@ -825,7 +825,7 @@ exprAnalyzeOrTerm(SrcList * pSrc,   /* the FROM clause */
  *   1.  The SQLITE_Transitive optimization must be enabled
  *   2.  Must be either an == or an IS operator
  *   3.  Not originating in the ON clause of an OUTER JOIN
- *   4.  The affinities of A and B must be compatible
+ *   4.  The types of A and B must be compatible
  *   5a. Both operands use the same collating sequence OR
  *   5b. The overall collating sequence is BINARY
  * If this routine returns TRUE, that means that the RHS can be substituted

>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
>> index d3a8644ce..b0a595b97 100644
>> --- a/src/box/sql/expr.c
>> +++ b/src/box/sql/expr.c
>> @@ -3213,7 +3226,9 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */
>>         if (rLhs != rLhsOrig)
>>                 sqlite3ReleaseTempReg(pParse, rLhs);
>>         sqlite3ExprCachePop(pParse);
>> +       sqlite3DbFree(pParse->db, aiMap);
>>         VdbeComment((v, "end IN expr"));
>> +       return;
> 
> 1. What was wrong with the previous way of the function finalization?
> I do not see any functional changes in this function, but logic of
> freeing is different. Why? Because zAff should not be freed? Then please,
> fix it in the previous commit. And to do not change logic of
> "sqlite3ExprCodeIN_finished:" you can nullify zAff after passing it to
> OP_ApplyType above.

@@ -3133,9 +3147,8 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */
         * of the RHS using the LHS as a probe.  If found, the result is
         * true.
         */
-       enum field_type *types = sql_affinity_str_to_field_type_str(zAff);
-       types[nVector] = field_type_MAX;
-       sqlite3VdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, (char *)types,
+       zAff[nVector] = field_type_MAX;
+       sqlite3VdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, (char*)zAff,

Before this patch, string (of types) passed to OP_ApplyType was allocated by
sql_affinity_str_to_field_type_str(), so it was different from zAff.
Now we pass directly zAff to OP_ApplyType, so we don’t need to free it.

On the other hand, indeed we can simply nullify it:

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index fbada57b0..a9640a3a1 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3150,6 +3150,11 @@ sqlite3ExprCodeIN(Parse * pParse,        /* Parsing and code generating context */
        zAff[nVector] = field_type_MAX;
        sqlite3VdbeAddOp4(v, OP_ApplyType, rLhs, nVector, 0, (char*)zAff,
                          P4_DYNAMIC);
+       /*
+        * zAff will be freed at the end of VDBE execution, since
+        * it was passed with P4_DYNAMIC flag.
+        */
+       zAff = NULL;
        if (destIfFalse == destIfNull) {
                /* Combine Step 3 and Step 5 into a single opcode */
                sqlite3VdbeAddOp4Int(v, OP_NotFound, pExpr->iTable,
@@ -3230,9 +3235,7 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */
        if (rLhs != rLhsOrig)
                sqlite3ReleaseTempReg(pParse, rLhs);
        sqlite3ExprCachePop(pParse);
-       sqlite3DbFree(pParse->db, aiMap);
        VdbeComment((v, "end IN expr"));
-       return;

>>   sqlite3ExprCodeIN_oom_error:
>>         sqlite3DbFree(pParse->db, aiMap);
>>         sqlite3DbFree(pParse->db, zAff);
>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index 209e7bf0f..822cc40df 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -1807,7 +1801,7 @@ struct Savepoint {
>>   * operator is NULL.  It is added to certain comparison operators to
>>   * prove that the operands are always NOT NULL.
>>   */
>> -#define SQLITE_KEEPNULL     0x08       /* Used by vector == or <> */
>> +#define SQLITE_KEEPNULL     0x40       /* Used by vector == or <> */
>>  #define SQLITE_JUMPIFNULL   0x10       /* jumps if either operand is NULL */
>>  #define SQLITE_STOREP2      0x20       /* Store result in reg[P2] rather than jump */
>>  #define SQLITE_NULLEQ       0x80       /* NULL=NULL */
> 
> 2. Please, keep them sorted. Cuts the eye.

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 850eea893..e3b5cac67 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1801,9 +1801,9 @@ struct Savepoint {
  * operator is NULL.  It is added to certain comparison operators to
  * prove that the operands are always NOT NULL.
  */
-#define SQLITE_KEEPNULL     0x40       /* Used by vector == or <> */
 #define SQLITE_JUMPIFNULL   0x10       /* jumps if either operand is NULL */
 #define SQLITE_STOREP2      0x20       /* Store result in reg[P2] rather than jump */
+#define SQLITE_KEEPNULL     0x40       /* Used by vector == or <> */
 #define SQLITE_NULLEQ       0x80       /* NULL=NULL */
 #define SQLITE_NOTNULL      0x90       /* Assert that operands are never NULL */

>> @@ -2615,7 +2609,8 @@ struct Select {
>>   */
>>  struct SelectDest {
>>         u8 eDest;               /* How to dispose of the results.  On of SRT_* above. */
>> -       char *zAffSdst;         /* Affinity used when eDest==SRT_Set */
>> +       /* Affinity used when eDest==SRT_Set */
>> +       enum field_type *zAffSdst;
> 
> 3. Not affinity.

@@ -2609,7 +2609,7 @@ struct Select {
  */
 struct SelectDest {
        u8 eDest;               /* How to dispose of the results.  On of SRT_* above. */
-       /* Affinity used when eDest==SRT_Set */
-        enum field_type *zAffSdst;
+       /** Type used when eDest==SRT_Set */
+       enum field_type *dest_type;
        int iSDParm;            /* A parameter used by the eDest disposal method */
        /** Register containing ephemeral's space pointer. */

>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index 61d73b676..c3f596d4f 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -2163,9 +2151,16 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
>>                         break;
>>                 }
>>         } else {
>> -               /* Neither operand is NULL.  Do a comparison. */
>> -               affinity = pOp->p5 & AFFINITY_MASK;
>> -               if (affinity>=AFFINITY_INTEGER) {
>> +               /*
>> +                * Neither operand is NULL. Do a comparison.
>> +                * 15 is 1111 in a binary form.
>> +                * Since all existing types can be fitted in 4 bits
>> +                * (field_type_MAX == 10), it is enough to 'and'
>> +                * p5 with this constant. Node that all other flags
>> +                * that can be stored in p5 are >= 16.
>> +                */
>> +               affinity = pOp->p5 & 15;
> 
> 4. Honestly, I did not expect it from you. Please, never use such
> constants and assumptions about them in code. field_type_MAX = 10 can
> be changed any day (for example, when we introduce decimal as a native
> Tarantool data type, or decide to add datetime). In fact, it is already
> not 10 - it is 9.

Oh, cmon. This place will explode only if number of types exceed 15.
Not so soon, I guess (when was the last time we introduced new type?).
So it won’t be my problem by the time it occurs.

> I propose you to either come up with a better solution or to use one of
> my solutions:
> 
> * return AFFINITY_MASK as P5_FIELD_TYPE_MASK, and add a static assertion
>  that P5_FIELD_TYPE_MASK < (1 << bit_count(field_type)).
> 
>  Or write in field_def.h something like this:
> 
>      static_assert(bit_count(field_type) <= 4,
>                    "size of enum field_type is used in "\
>                    "VdbeOp.p5 to fit it into 4 bits”);

To be honest I don’t understand these bit assertions.
Why can’t we simply check that field_type_MAX <= 15?

diff --git a/src/box/field_def.c b/src/box/field_def.c
index b34d2ccd9..1a8074650 100644
--- a/src/box/field_def.c
+++ b/src/box/field_def.c
@@ -33,6 +33,13 @@
 #include "trivia/util.h"
 #include "key_def.h"
 
+/**
+ * For detailed explanation see context of OP_Eq, OP_Lt etc
+ * opcodes in vdbe.c.
+ */
+static_assert(field_type_MAX <= 15, "values of enum field_type "\
+             "should fit into 4 bits of VdbeOp.p5");
+

Also, as you may notice, I placed it to field_def.c
Seems that field_def.h is used to generate module.h,
which in turn doesn’t tolerate this kind of assert.

>  To get bit_count during compilation you can specify it explicitly right
>  after field_type_MAX in enum field_type so as to change it always
>  together with field_type_MAX.
> 
>> diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
>> index 33b860f36..01c1a2c6c 100644
>> --- a/src/box/sql/wherecode.c
>> +++ b/src/box/sql/wherecode.c
>> @@ -367,15 +367,15 @@ disableTerm(WhereLevel * pLevel, WhereTerm * pTerm)
>>   * Code an OP_ApplyType opcode to apply the column type string types
>>   * to the n registers starting at base.
>>   *
>> - * As an optimization, AFFINITY_BLOB entries (which are no-ops) at the
>> + * As an optimization, SCALAR entries (which are no-ops) at the
>>   * beginning and end of zAff are ignored.  If all entries in zAff are
>> - * AFFINITY_BLOB, then no code gets generated.
>> + * SCALAR, then no code gets generated.
>>   *
>>   * This routine makes its own copy of zAff so that the caller is free
>>   * to modify zAff after this routine returns.
>>   */
>>  static void
>> -codeApplyAffinity(Parse * pParse, int base, int n, char *zAff)
>> +codeApplyAffinity(Parse * pParse, int base, int n, enum field_type *zAff)
> 
> 5. Not affinity.

Ok, refactored whole function:

diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 53445faf5..1f28893f6 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -363,22 +363,22 @@ disableTerm(WhereLevel * pLevel, WhereTerm * pTerm)
        }
 }
 
-/*
- * Code an OP_ApplyType opcode to apply the column type string types
- * to the n registers starting at base.
+/**
+ * Code an OP_ApplyType opcode to apply the column type string
+ * @types to the n registers starting at @base.
  *
  * As an optimization, SCALAR entries (which are no-ops) at the
- * beginning and end of zAff are ignored.  If all entries in zAff are
- * SCALAR, then no code gets generated.
+ * beginning and end of @types are ignored.  If all entries in
+ * @types are SCALAR, then no code gets generated.
  *
- * This routine makes its own copy of zAff so that the caller is free
- * to modify zAff after this routine returns.
+ * This routine makes its own copy of @types so that the caller is
+ * free to modify @types after this routine returns.
  */
 static void
-codeApplyAffinity(Parse * pParse, int base, int n, enum field_type *zAff)
+emit_apply_type(Parse *pParse, int base, int n, enum field_type *types)
 {
        Vdbe *v = pParse->pVdbe;
-       if (zAff == 0) {
+       if (types == NULL) {
                assert(pParse->db->mallocFailed);
                return;
        }
@@ -388,17 +388,17 @@ codeApplyAffinity(Parse * pParse, int base, int n, enum field_type *zAff)
         * Adjust base and n to skip over SCALAR entries at the
         * beginning and end of the type sequence.
         */
-       while (n > 0 && zAff[0] == FIELD_TYPE_SCALAR) {
+       while (n > 0 && types[0] == FIELD_TYPE_SCALAR) {
                n--;
                base++;
-               zAff++;
+               types++;
        }
-       while (n > 1 && zAff[n - 1] == FIELD_TYPE_SCALAR) {
+       while (n > 1 && types[n - 1] == FIELD_TYPE_SCALAR) {
                n--;
        }
 
        if (n > 0) {
-               enum field_type *types = field_type_sequence_dup(pParse, zAff,
-                                                                n);
-               sqlite3VdbeAddOp4(v, OP_ApplyType, base, n, 0, (char *) types,
-                                 P4_DYNAMIC);
+               enum field_type *types_dup = field_type_sequence_dup(pParse,
+                                                                    types, n);
+               sqlite3VdbeAddOp4(v, OP_ApplyType, base, n, 0,
+                                 (char *) types_dup, P4_DYNAMIC);
@@ -1190,8 +1190,8 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,  /* Complete information about t
                                }
                        }
                }
-               codeApplyAffinity(pParse, regBase, nConstraint - bSeekPastNull,
-                                 zStartAff);
+               emit_apply_type(pParse, regBase, nConstraint - bSeekPastNull,
+                               zStartAff);
                if (pLoop->nSkip > 0 && nConstraint == pLoop->nSkip) {
                        /* The skip-scan logic inside the call to codeAllEqualityConstraints()
                         * above has already left the cursor sitting on the correct row,
@@ -1243,8 +1243,8 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,  /* Complete information about t
                        }
                        if (zEndAff) {
                                updateRangeAffinityStr(pRight, nTop, zEndAff);
-                               codeApplyAffinity(pParse, regBase + nEq, nTop,
-                                                 zEndAff);
+                               emit_apply_type(pParse, regBase + nEq, nTop,
+                                               zEndAff);
                        } else {
                                assert(pParse->db->mallocFailed);
                        }

>> @@ -419,16 +419,15 @@ codeApplyAffinity(Parse * pParse, int base, int n, char *zAff)
>>  static void
>>  updateRangeAffinityStr(Expr * pRight,  /* RHS of comparison */
>>                        int n,           /* Number of vector elements in comparison */
>> -                      char *zAff)      /* Affinity string to modify */
>> +                      enum field_type *zAff)   /* Affinity string to modify */
> 
> 6. The same. In many other places too. Where you changed type and
> usages of a variable, but kept its name.

I did it on purpose - to reduce diff, so that separate functional
changes and non-functional such as renaming. Now fixed.





More information about the Tarantool-patches mailing list