[Tarantool-patches] [PATCH v5 08/52] sql: disable unused code in sql/vdbemem.c

imeevma at tarantool.org imeevma at tarantool.org
Fri Apr 9 19:59:51 MSK 2021


Thank you for the review! My answer and new patch below.



On 30.03.2021 01:58, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> I didn't even know we have vdbemem.c. It should be deleted someday
> when we have mem.h/mem.c instead.
>
>> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
>> index dd8163f5e..b4293d961 100644
>> --- a/src/box/sql/sqlInt.h
>> +++ b/src/box/sql/sqlInt.h
>> @@ -4151,31 +4151,54 @@ sql_expr_new_column(struct sql *db, struct SrcList *src_list, int src_idx,
>>  
>>  int sqlExprCheckIN(Parse *, Expr *);
>>  
>> -int sqlStat4ProbeSetValue(Parse *, struct index_def *, UnpackedRecord **, Expr *, int,
>> -			      int, int *);
>> -int sqlStat4ValueFromExpr(Parse *, Expr *, enum field_type type,
>> -			      sql_value **);
>> -void sqlStat4ProbeFree(UnpackedRecord *);
>> +/* TODO: Enable this function when stat-tables will be revived. */
>> +static inline int
>> +sqlStat4ProbeSetValue(struct Parse *parse, struct index_def *def,
>> +		      struct UnpackedRecord **rec, struct Expr *expr, int n,
>> +		      int i, int *out)
>
> You can also make it take '...' instead of all of these arguments individually.
> The same for the other functions.
>
Thank you! Dixed.

>> +{
>> +	(void)parse;
>> +	(void)def;
>> +	(void)rec;
>> +	(void)expr;
>> +	(void)n;
>> +	(void)i;
>> +	(void)out;
>> +	unreachable();
>> +	return 0;
>> +}



New patch:

commit 152edbab6445a2b77f06c2164310a00a335b6df4
Author: Mergen Imeev <imeevma at gmail.com>
Date:   Thu Apr 1 15:09:15 2021 +0300

    sql: disable unused code in sql/vdbemem.c
    
    This patch disables unused code in sql/vdbemem.c. It will simplify
    refactoring.
    
    Part of #5818

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index dd8163f5e..e075224c6 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4151,31 +4151,38 @@ sql_expr_new_column(struct sql *db, struct SrcList *src_list, int src_idx,
 
 int sqlExprCheckIN(Parse *, Expr *);
 
-int sqlStat4ProbeSetValue(Parse *, struct index_def *, UnpackedRecord **, Expr *, int,
-			      int, int *);
-int sqlStat4ValueFromExpr(Parse *, Expr *, enum field_type type,
-			      sql_value **);
-void sqlStat4ProbeFree(UnpackedRecord *);
+/* TODO: Enable this function when stat-tables will be revived. */
+static inline int
+sqlStat4ProbeSetValue(struct Parse *parse, ...)
+{
+	(void)parse;
+	unreachable();
+	return 0;
+}
 
-/**
- * Extract the col_num-th column from the record.  Write
- * the column value into *res.  If *res is initially NULL
- * then a new sql_value object is allocated.
- *
- * If *res is initially NULL then the caller is responsible for
- * ensuring that the value written into *res is eventually
- * freed.
- *
- * @param db Database handle.
- * @param record Pointer to buffer containing record.
- * @param col_num Column to extract.
- * @param[out] res Extracted value.
- *
- * @retval -1 on error or 0.
- */
-int
-sql_stat4_column(struct sql *db, const char *record, uint32_t col_num,
-		 sql_value **res);
+/* TODO: Enable this function when stat-tables will be revived. */
+static inline int
+sqlStat4ValueFromExpr(struct Parse *parse, ...)
+{
+	(void)parse;
+	unreachable();
+	return 0;
+}
+
+/* TODO: Enable this function when stat-tables will be revived. */
+static inline void
+sqlStat4ProbeFree(struct UnpackedRecord *rec)
+{
+	(void)rec;
+}
+
+/* TODO: Enable this function when stat-tables will be revived. */
+static inline int
+sql_stat4_column(struct sql *db, ...)
+{
+	(void)db;
+	return 0;
+}
 
 /*
  * The interface to the LEMON-generated parser
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a4a8c45f2..e269857ea 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -58,16 +58,30 @@
 #include "box/sequence.h"
 #include "box/session_settings.h"
 
+#ifdef SQL_DEBUG
+
 /*
- * Invoke this macro on memory cells just prior to changing the
- * value of the cell.  This macro verifies that shallow copies are
- * not misused.  A shallow copy of a string or blob just copies a
- * pointer to the string or blob, not the content.  If the original
- * is changed while the copy is still in use, the string or blob might
- * be changed out from under the copy.  This macro verifies that nothing
- * like that ever happens.
+ * This routine prepares a memory cell for modification by breaking
+ * its link to a shallow copy and by marking any current shallow
+ * copies of this cell as invalid.
+ *
+ * This is used for testing and debugging only - to make sure shallow
+ * copies are not misused.
  */
-#ifdef SQL_DEBUG
+static void
+sqlVdbeMemAboutToChange(Vdbe * pVdbe, Mem * pMem)
+{
+	int i;
+	Mem *pX;
+	for (i = 0, pX = pVdbe->aMem; i < pVdbe->nMem; i++, pX++) {
+		if (pX->pScopyFrom == pMem) {
+			pX->flags |= MEM_Undefined;
+			pX->pScopyFrom = 0;
+		}
+	}
+	pMem->pScopyFrom = 0;
+}
+
 # define memAboutToChange(P,M) sqlVdbeMemAboutToChange(P,M)
 #else
 # define memAboutToChange(P,M)
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 541b93d30..b48e45770 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -362,10 +362,6 @@ int sqlVdbeSorterRewind(const VdbeCursor *, int *);
 int sqlVdbeSorterWrite(const VdbeCursor *, Mem *);
 int sqlVdbeSorterCompare(const VdbeCursor *, Mem *, int, int *);
 
-#ifdef SQL_DEBUG
-void sqlVdbeMemAboutToChange(Vdbe *, Mem *);
-#endif
-
 int sqlVdbeCheckFk(Vdbe *, int);
 
 int sqlVdbeMemTranslate(Mem *, u8);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index d977cbac8..263fe5b00 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -44,29 +44,8 @@
 #include "box/tuple.h"
 #include "mpstream/mpstream.h"
 
-#ifdef SQL_DEBUG
-/*
- * This routine prepares a memory cell for modification by breaking
- * its link to a shallow copy and by marking any current shallow
- * copies of this cell as invalid.
- *
- * This is used for testing and debugging only - to make sure shallow
- * copies are not misused.
- */
-void
-sqlVdbeMemAboutToChange(Vdbe * pVdbe, Mem * pMem)
-{
-	int i;
-	Mem *pX;
-	for (i = 0, pX = pVdbe->aMem; i < pVdbe->nMem; i++, pX++) {
-		if (pX->pScopyFrom == pMem) {
-			pX->flags |= MEM_Undefined;
-			pX->pScopyFrom = 0;
-		}
-	}
-	pMem->pScopyFrom = 0;
-}
-#endif				/* SQL_DEBUG */
+#if 0
+
 /*
  * Context object passed by sqlStat4ProbeSetValue() through to
  * valueNew(). See comments above valueNew() for details.
@@ -546,6 +525,22 @@ sqlStat4ValueFromExpr(Parse * pParse,	/* Parse context */
 	return stat4ValueFromExpr(pParse, pExpr, type, 0, ppVal);
 }
 
+/**
+ * Extract the col_num-th column from the record.  Write
+ * the column value into *res.  If *res is initially NULL
+ * then a new sql_value object is allocated.
+ *
+ * If *res is initially NULL then the caller is responsible for
+ * ensuring that the value written into *res is eventually
+ * freed.
+ *
+ * @param db Database handle.
+ * @param record Pointer to buffer containing record.
+ * @param col_num Column to extract.
+ * @param[out] res Extracted value.
+ *
+ * @retval -1 on error or 0.
+ */
 int
 sql_stat4_column(struct sql *db, const char *record, uint32_t col_num,
 		 sql_value **res)
@@ -588,3 +583,5 @@ sqlStat4ProbeFree(UnpackedRecord * pRec)
 		sqlDbFree(aMem[0].db, pRec);
 	}
 }
+
+#endif


More information about the Tarantool-patches mailing list