Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: v.shpilevoy@tarantool.org, tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v1 18/21] sql: remove unused code
Date: Mon, 25 Oct 2021 11:51:53 +0300
Message-ID: <20211025085153.GL36295@tarantool.org> (raw)
In-Reply-To: <431c5ea816ff1264a0672d54e25fb2a8a22a6c20.1633713432.git.imeevma@gmail.com>

Thank you for the review! I removed sql/utf.c module, dropped one unused field
of struct Vdbe and removed unused prototype. Diff below.

On Fri, Oct 08, 2021 at 08:32:05PM +0300, Mergen Imeev via Tarantool-patches wrote:
<cut>

Diff:

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index a6de36c66..e578461ef 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -55,7 +55,6 @@ set(sql_sources
     sql/tokenize.c
     sql/treeview.c
     sql/trigger.c
-    sql/utf.c
     sql/update.c
     sql/util.c
     sql/vdbe.c
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 294441e5c..9533833f2 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -823,13 +823,6 @@ mem_len_unsafe(const struct Mem *mem)
 	return len;
 }
 
-/**
- * Return address of memory allocated for accumulation structure of the
- * aggregate function.
- */
-int
-mem_get_agg(const struct Mem *mem, void **accum);
-
 /**
  * Simple type to str convertor. It is used to simplify
  * error reporting.
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 2bbf281ee..22a4aa5cd 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3652,26 +3652,6 @@ void sqlDetach(Parse *, Expr *);
 int sqlAtoF(const char *z, double *, int);
 int sqlGetInt32(const char *, int *);
 
-/**
- * Return number of symbols in the given string.
- *
- * Number of symbols != byte size of string because some symbols
- * are encoded with more than one byte. Also note that all
- * symbols from 'str' to 'str + byte_len' would be counted,
- * even if there is a '\0' somewhere between them.
- *
- * This function is implemented to be fast and indifferent to
- * correctness of string being processed. If input string has
- * even one invalid utf-8 sequence, then the resulting length
- * could be arbitary in these boundaries (0 < len < byte_len).
- * @param str String to be counted.
- * @param byte_len Byte length of given string.
- * @return number of symbols in the given string.
- */
-int
-sql_utf8_char_count(const unsigned char *str, int byte_len);
-
-u32 sqlUtf8Read(const u8 **);
 LogEst sqlLogEst(u64);
 LogEst sqlLogEstAdd(LogEst, LogEst);
 u64 sqlLogEstToInt(LogEst);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 7c983fea5..fbd159126 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -805,11 +805,8 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger,
 
 		if (!parser->is_aborted)
 			parser->is_aborted = pSubParse->is_aborted;
-		if (db->mallocFailed == 0) {
-			pProgram->aOp =
-			    sqlVdbeTakeOpArray(v, &pProgram->nOp,
-						   &pTop->nMaxArg);
-		}
+		if (db->mallocFailed == 0)
+			pProgram->aOp = sqlVdbeTakeOpArray(v, &pProgram->nOp);
 		pProgram->nMem = pSubParse->nMem;
 		pProgram->nCsr = pSubParse->nTab;
 		pProgram->token = (void *)trigger;
diff --git a/src/box/sql/utf.c b/src/box/sql/utf.c
deleted file mode 100644
index 2d9a12806..000000000
--- a/src/box/sql/utf.c
+++ /dev/null
@@ -1,95 +0,0 @@
-/*
- * Copyright 2010-2017, Tarantool AUTHORS, please see AUTHORS file.
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * 1. Redistributions of source code must retain the above
- *    copyright notice, this list of conditions and the
- *    following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above
- *    copyright notice, this list of conditions and the following
- *    disclaimer in the documentation and/or other materials
- *    provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
- * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
- * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
- * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
- * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-/*
- * This file contains routines used to translate between UTF-8.
- *
- * Notes on UTF-8:
- *
- *   Byte-0    Byte-1    Byte-2    Byte-3    Value
- *  0xxxxxxx                                 00000000 00000000 0xxxxxxx
- *  110yyyyy  10xxxxxx                       00000000 00000yyy yyxxxxxx
- *  1110zzzz  10yyyyyy  10xxxxxx             00000000 zzzzyyyy yyxxxxxx
- *  11110uuu  10uuzzzz  10yyyyyy  10xxxxxx   000uuuuu zzzzyyyy yyxxxxxx
- *
- *
- */
-#include "sqlInt.h"
-
-/*
- * This lookup table is used to help decode the first byte of
- * a multi-byte UTF8 character.
- */
-static const unsigned char sqlUtf8Trans1[] = {
-	0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
-	0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
-	0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
-	0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f,
-	0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
-	0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
-	0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
-	0x00, 0x01, 0x02, 0x03, 0x00, 0x01, 0x00, 0x00,
-};
-
-u32
-sqlUtf8Read(const unsigned char **pz	/* Pointer to string from which to read char */
-    )
-{
-	unsigned int c;
-
-	/* Same as READ_UTF8() above but without the zTerm parameter.
-	 * For this routine, we assume the UTF8 string is always zero-terminated.
-	 */
-	c = *((*pz)++);
-	if (c >= 0xc0) {
-		c = sqlUtf8Trans1[c - 0xc0];
-		while ((*(*pz) & 0xc0) == 0x80) {
-			c = (c << 6) + (0x3f & *((*pz)++));
-		}
-		if (c < 0x80
-		    || (c & 0xFFFFF800) == 0xD800
-		    || (c & 0xFFFFFFFE) == 0xFFFE) {
-			c = 0xFFFD;
-		}
-	}
-	return c;
-}
-
-int
-sql_utf8_char_count(const unsigned char *str, int byte_len)
-{
-	int symbol_count = 0;
-	for (int i = 0; i < byte_len;) {
-		SQL_UTF8_FWD_1(str, i, byte_len);
-		symbol_count++;
-	}
-	return symbol_count;
-}
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index e40a1a0b3..e482926b2 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -260,7 +260,10 @@ void sqlVdbeCountChanges(Vdbe *);
 sql *sqlVdbeDb(Vdbe *);
 void sqlVdbeSetSql(Vdbe *, const char *z, int n);
 void sqlVdbeSwap(Vdbe *, Vdbe *);
-VdbeOp *sqlVdbeTakeOpArray(Vdbe *, int *, int *);
+
+struct VdbeOp *
+sqlVdbeTakeOpArray(struct Vdbe *p, int *pnOp);
+
 sql_value *sqlVdbeGetBoundValue(Vdbe *, int);
 char *sqlVdbeExpandSql(Vdbe *, const char *);
 
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 8dbba4908..c45cc9301 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -262,7 +262,6 @@ struct Vdbe {
 
 	Op *aOp;		/* Space to hold the virtual machine's program */
 	Mem *aMem;		/* The memory locations */
-	Mem **apArg;		/* Arguments to currently executing user function */
 	/** SQL metadata for DML/DQL queries. */
 	struct sql_column_metadata *metadata;
 	Mem *pResultSet;	/* Pointer to an array of results */
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 3015760e1..f5a84956c 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -447,21 +447,17 @@ sqlVdbeRunOnlyOnce(Vdbe * p)
  * (1) For each jump instruction with a negative P2 value (a label)
  *     resolve the P2 value to an actual address.
  *
- * (2) Compute the maximum number of arguments used by any SQL function
- *     and store that value in *pMaxFuncArgs.
+ * (2) Initialize the p4.xAdvance pointer on opcodes that use it.
  *
- * (3) Initialize the p4.xAdvance pointer on opcodes that use it.
- *
- * (4) Reclaim the memory allocated for storing labels.
+ * (3) Reclaim the memory allocated for storing labels.
  *
  * This routine will only function correctly if the mkopcodeh.sh generator
  * script numbers the opcodes correctly.  Changes to this routine must be
  * coordinated with changes to mkopcodeh.sh.
  */
 static void
-resolveP2Values(Vdbe * p, int *pMaxFuncArgs)
+resolveP2Values(Vdbe * p)
 {
-	int nMaxArgs = *pMaxFuncArgs;
 	Op *pOp;
 	Parse *pParse = p->pParse;
 	int *aLabel = pParse->aLabel;
@@ -506,7 +502,6 @@ resolveP2Values(Vdbe * p, int *pMaxFuncArgs)
 	sqlDbFree(p->db, pParse->aLabel);
 	pParse->aLabel = 0;
 	pParse->nLabel = 0;
-	*pMaxFuncArgs = nMaxArgs;
 }
 
 /*
@@ -526,17 +521,15 @@ sqlVdbeCurrentAddr(Vdbe * p)
  * vdbeFreeOpArray() function.
  *
  * Before returning, *pnOp is set to the number of entries in the returned
- * array. Also, *pnMaxArg is set to the larger of its current value and
- * the number of entries in the Vdbe.apArg[] array required to execute the
- * returned program.
+ * array.
  */
-VdbeOp *
-sqlVdbeTakeOpArray(Vdbe * p, int *pnOp, int *pnMaxArg)
+struct VdbeOp *
+sqlVdbeTakeOpArray(struct Vdbe *p, int *pnOp)
 {
 	VdbeOp *aOp = p->aOp;
 	assert(aOp && !p->db->mallocFailed);
 
-	resolveP2Values(p, pnMaxArg);
+	resolveP2Values(p);
 	*pnOp = p->nOp;
 	p->aOp = 0;
 	return aOp;
@@ -1485,7 +1478,6 @@ sqlVdbeMakeReady(Vdbe * p,	/* The VDBE */
 	int nVar;		/* Number of parameters */
 	int nMem;		/* Number of VM memory registers */
 	int nCursor;		/* Number of cursors required */
-	int nArg;		/* Number of arguments in subprograms */
 	int n;			/* Loop counter */
 	struct ReusableSpace x;	/* Reusable bulk memory */
 
@@ -1499,7 +1491,6 @@ sqlVdbeMakeReady(Vdbe * p,	/* The VDBE */
 	nVar = pParse->nVar;
 	nMem = pParse->nMem;
 	nCursor = pParse->nTab;
-	nArg = pParse->nMaxArg;
 
 	/* Each cursor uses a memory cell.  The first cursor (cursor 0) can
 	 * use aMem[0] which is not otherwise used by the VDBE program.  Allocate
@@ -1521,7 +1512,7 @@ sqlVdbeMakeReady(Vdbe * p,	/* The VDBE */
 	assert(x.nFree >= 0);
 	assert(EIGHT_BYTE_ALIGNMENT(&x.pSpace[x.nFree]));
 
-	resolveP2Values(p, &nArg);
+	resolveP2Values(p);
 	if (pParse->explain && nMem < 10) {
 		nMem = 10;
 	}
@@ -1541,7 +1532,6 @@ sqlVdbeMakeReady(Vdbe * p,	/* The VDBE */
 		x.nNeeded = 0;
 		p->aMem = allocSpace(&x, p->aMem, nMem * sizeof(Mem));
 		p->aVar = allocSpace(&x, p->aVar, nVar * sizeof(Mem));
-		p->apArg = allocSpace(&x, p->apArg, nArg * sizeof(Mem *));
 		p->apCsr =
 		    allocSpace(&x, p->apCsr, nCursor * sizeof(VdbeCursor *));
 		if (x.nNeeded == 0)

  reply	other threads:[~2021-10-25  8:51 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08 17:31 [Tarantool-patches] [PATCH v1 00/21] Refactor non-standard and non-aggragate functions Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 01/21] sql: refactor CHAR() function Mergen Imeev via Tarantool-patches
2021-10-14 22:42   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:02     ` Mergen Imeev via Tarantool-patches
2021-10-29 23:42       ` Vladislav Shpilevoy via Tarantool-patches
2021-11-02 11:35         ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 02/21] sql: refactor GREATEST() and LEAST() functions Mergen Imeev via Tarantool-patches
2021-10-14 22:42   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:17     ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 03/21] sql: refactor HEX() function Mergen Imeev via Tarantool-patches
2021-10-14 22:43   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:19     ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 04/21] sql: refactor LENGTH() function Mergen Imeev via Tarantool-patches
2021-10-14 22:43   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:30     ` Mergen Imeev via Tarantool-patches
2021-10-29 23:42       ` Vladislav Shpilevoy via Tarantool-patches
2021-11-02 11:39         ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 05/21] sql: refactor PRINTF() function Mergen Imeev via Tarantool-patches
2021-10-14 22:44   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:33     ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 06/21] sql: refactor RANDOM() function Mergen Imeev via Tarantool-patches
2021-10-25  8:35   ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 07/21] sql: rework RANDOMBLOB() function Mergen Imeev via Tarantool-patches
2021-10-25  8:36   ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 08/21] sql: refactor ZEROBLOB() function Mergen Imeev via Tarantool-patches
2021-10-25  8:37   ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 09/21] sql: refactor TYPEOF() function Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 10/21] sql: refactor ROUND() function Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 11/21] sql: refactor ROW_COUNT() function Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 12/21] sql: rework UUID() function Mergen Imeev via Tarantool-patches
2021-10-25  8:38   ` Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 13/21] sql: refactor VERSION() function Mergen Imeev via Tarantool-patches
2021-10-08 17:31 ` [Tarantool-patches] [PATCH v1 14/21] sql: refactor UNICODE() function Mergen Imeev via Tarantool-patches
2021-10-14 22:44   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:40     ` Mergen Imeev via Tarantool-patches
2021-11-02 11:42       ` Mergen Imeev via Tarantool-patches
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 15/21] sql: refactor of SOUNDEX() function Mergen Imeev via Tarantool-patches
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 16/21] sql: refactor REPLACE() function Mergen Imeev via Tarantool-patches
2021-10-14 22:45   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:45     ` Mergen Imeev via Tarantool-patches
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 17/21] sql: refactor QUOTE() function Mergen Imeev via Tarantool-patches
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 18/21] sql: remove unused code Mergen Imeev via Tarantool-patches
2021-10-25  8:51   ` Mergen Imeev via Tarantool-patches [this message]
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 19/21] sql: remove MEM_Dyn flag Mergen Imeev via Tarantool-patches
2021-10-14 22:46   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  8:54     ` Mergen Imeev via Tarantool-patches
2021-10-29 23:43       ` Vladislav Shpilevoy via Tarantool-patches
2021-11-02 11:43         ` Mergen Imeev via Tarantool-patches
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 20/21] sql: remove MEM_Term flag Mergen Imeev via Tarantool-patches
2021-10-14 22:47   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-25  9:57     ` Mergen Imeev via Tarantool-patches
2021-10-08 17:32 ` [Tarantool-patches] [PATCH v1 21/21] sql: make arguments to be const Mergen Imeev via Tarantool-patches
2021-11-02 22:15 ` [Tarantool-patches] [PATCH v1 00/21] Refactor non-standard and non-aggragate functions Vladislav Shpilevoy via Tarantool-patches
2021-11-11 10:48 Mergen Imeev via Tarantool-patches
2021-11-11 10:49 ` [Tarantool-patches] [PATCH v1 18/21] sql: remove unused code Mergen Imeev via Tarantool-patches

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=20211025085153.GL36295@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /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

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git