From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 7/8] sql: clean-up affinity from SQL source code
Date: Mon, 28 Jan 2019 19:40:05 +0300 [thread overview]
Message-ID: <33150EBA-AB14-4089-8399-0FBD07C71BD8@tarantool.org> (raw)
In-Reply-To: <2cd04a2d-6cd8-9e0b-7d51-d9431b6c5d82@tarantool.org>
>> 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.
next prev parent reply other threads:[~2019-01-28 16:40 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-28 9:34 [tarantool-patches] [PATCH 0/8] Eliminate affinity from " Nikita Pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 1/8] sql: remove SQLITE_ENABLE_UPDATE_DELETE_LIMIT define Nikita Pettik
2018-12-29 17:42 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-16 14:25 ` n.pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 2/8] sql: use field type instead of affinity for type_def Nikita Pettik
2018-12-29 17:42 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-16 14:26 ` n.pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 3/8] sql: remove numeric affinity Nikita Pettik
2018-12-29 9:01 ` [tarantool-patches] " Konstantin Osipov
2018-12-29 17:42 ` Vladislav Shpilevoy
2019-01-09 8:26 ` Konstantin Osipov
2019-01-16 14:26 ` n.pettik
2019-01-22 15:41 ` Vladislav Shpilevoy
2019-01-28 16:39 ` n.pettik
2019-01-30 13:04 ` Vladislav Shpilevoy
2019-02-01 16:39 ` n.pettik
2019-01-09 8:20 ` Konstantin Osipov
2018-12-28 9:34 ` [tarantool-patches] [PATCH 4/8] sql: replace affinity with field type for func Nikita Pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 5/8] sql: replace field type with affinity for VDBE runtime Nikita Pettik
2018-12-29 17:42 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-16 14:26 ` n.pettik
2019-01-22 15:41 ` Vladislav Shpilevoy
2019-01-28 16:39 ` n.pettik
2019-01-30 13:04 ` Vladislav Shpilevoy
2019-02-01 16:39 ` n.pettik
2019-02-05 15:08 ` Vladislav Shpilevoy
2019-02-05 17:46 ` n.pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 6/8] sql: replace affinity with field type in struct Expr Nikita Pettik
2018-12-29 17:42 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-16 14:26 ` n.pettik
2019-01-22 15:41 ` Vladislav Shpilevoy
2019-01-28 16:39 ` n.pettik
2019-01-30 13:04 ` Vladislav Shpilevoy
2019-02-01 16:39 ` n.pettik
2019-02-05 15:08 ` Vladislav Shpilevoy
2019-02-05 17:46 ` n.pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 7/8] sql: clean-up affinity from SQL source code Nikita Pettik
2018-12-29 17:42 ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-16 14:26 ` n.pettik
2019-01-22 15:41 ` Vladislav Shpilevoy
2019-01-28 16:40 ` n.pettik [this message]
2019-01-30 13:04 ` Vladislav Shpilevoy
2019-02-01 16:39 ` n.pettik
2019-02-05 15:08 ` Vladislav Shpilevoy
2019-02-05 17:46 ` n.pettik
2018-12-28 9:34 ` [tarantool-patches] [PATCH 8/8] Remove affinity from field definition Nikita Pettik
2019-02-05 19:41 ` [tarantool-patches] Re: [PATCH 0/8] Eliminate affinity from source code Vladislav Shpilevoy
2019-02-08 13:37 ` 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=33150EBA-AB14-4089-8399-0FBD07C71BD8@tarantool.org \
--to=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=v.shpilevoy@tarantool.org \
--subject='[tarantool-patches] Re: [PATCH 7/8] sql: clean-up affinity from SQL source code' \
/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