[tarantool-patches] Re: [PATCH v1 25/28] sql: remove sql_log()

[Mergen Imeev]

On Fri, Jun 14, 2019 at 12:24:53AM +0200, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> Consider my review fixes below and on the branch
> in a separate commit.
> 
Thank you! New patch:

>From 59aeab3a41c3107bccbcb31f9f9b1bc7f8f9ed81 Mon Sep 17 00:00:00 2001
Date: Sat, 25 May 2019 11:56:19 +0300
Subject: [PATCH] sql: remove sql_log()

This function is not used in Tarantool and should be removed.

diff --git a/src/box/sql/global.c b/src/box/sql/global.c
index 13e9c67..2d937d7 100644
--- a/src/box/sql/global.c
+++ b/src/box/sql/global.c
@@ -196,8 +196,6 @@ SQL_WSD struct sqlConfig sqlConfig = {
 	0,			/* isInit */
 	0,			/* inProgress */
 	0,			/* isMallocInit */
-	0,			/* xLog */
-	0,			/* pLogArg */
 #ifdef SQL_VDBE_COVERAGE
 	0,			/* xVdbeBranch */
 	0,			/* pVbeBranchArg */
diff --git a/src/box/sql/os_unix.c b/src/box/sql/os_unix.c
index ad8f289..683a71d 100644
--- a/src/box/sql/os_unix.c
+++ b/src/box/sql/os_unix.c
@@ -1073,10 +1073,9 @@ unixUnmapfile(unixFile * pFd)
  *       unixFile.mmapSize
  *       unixFile.mmapSizeActual
  *
- * If unsuccessful, an error message is logged via sql_log() and
- * the three variables above are zeroed. In this case sql should
- * continue accessing the database using the xRead() and xWrite()
- * methods.
+ * If unsuccessful,the three variables above are zeroed. In this
+ * case sql should continue accessing the database using the
+ * xRead() and xWrite() methods.
  */
 static void
 unixRemapfile(unixFile * pFd,	/* File descriptor object */
diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
index 8abb673..98372f0 100644
--- a/src/box/sql/printf.c
+++ b/src/box/sql/printf.c
@@ -1114,40 +1114,6 @@ sql_snprintf(int n, char *zBuf, const char *zFormat, ...)
 	return z;
 }
 
-/*
- * This is the routine that actually formats the sql_log() message.
- * We house it in a separate routine from sql_log() to avoid using
- * stack space on small-stack systems when logging is disabled.
- *
- * sqlVXPrintf() might ask for *temporary* memory allocations for
- * certain format characters (%q) or for very large precisions or widths.
- */
-static void
-renderLogMsg(int iErrCode, const char *zFormat, va_list ap)
-{
-	StrAccum acc;		/* String accumulator */
-	char zMsg[SQL_PRINT_BUF_SIZE * 3];	/* Complete log message */
-
-	sqlStrAccumInit(&acc, 0, zMsg, sizeof(zMsg), 0);
-	sqlVXPrintf(&acc, zFormat, ap);
-	sqlGlobalConfig.xLog(sqlGlobalConfig.pLogArg, iErrCode,
-				 sqlStrAccumFinish(&acc));
-}
-
-/*
- * Format and write a message to the log if logging is enabled.
- */
-void
-sql_log(int iErrCode, const char *zFormat, ...)
-{
-	va_list ap;		/* Vararg list */
-	if (sqlGlobalConfig.xLog) {
-		va_start(ap, zFormat);
-		renderLogMsg(iErrCode, zFormat, ap);
-		va_end(ap);
-	}
-}
-
 #if defined(SQL_DEBUG)
 /*
  * A version of printf() that understands %lld.  Used for debugging.
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index be8cc25..fdf3703 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -363,8 +363,7 @@ lookupName(Parse * pParse,	/* The parsing context */
 		 * The ability to use an output result-set column in the WHERE, GROUP BY,
 		 * or HAVING clauses, or as part of a larger expression in the ORDER BY
 		 * clause is not standard SQL.  This is a (goofy) sql extension, that
-		 * is supported for backwards compatibility only. Hence, we issue a warning
-		 * on sql_log() whenever the capability is used.
+		 * is supported for backwards compatibility only.
 		 */
 		if ((pEList = pNC->pEList) != 0 && zTab == 0 && cnt == 0) {
 			for (j = 0; j < pEList->nExpr; j++) {
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index c7a1a81..434a5bd 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -554,9 +554,6 @@ sql_row_count(struct sql_context *context, MAYBE_UNUSED int unused1,
 void *
 sql_user_data(sql_context *);
 
-void
-sql_log(int iErrCode, const char *zFormat, ...);
-
 void *
 sql_aggregate_context(sql_context *,
 			  int nBytes);
@@ -2592,8 +2589,6 @@ struct sqlConfig {
 	int isInit;		/* True after initialization has finished */
 	int inProgress;		/* True while initialization in progress */
 	int isMallocInit;	/* True after malloc is initialized */
-	void (*xLog) (void *, int, const char *);	/* Function for logging */
-	void *pLogArg;		/* First argument to xLog() */
 #ifdef SQL_VDBE_COVERAGE
 	/* The following callback (if not NULL) is invoked on every VDBE branch
 	 * operation.  Set the callback using sql_TESTCTRL_VDBE_COVERAGE.