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
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v4 03/16] sql: use register P1 for number of arguments
Date: Fri,  1 Oct 2021 15:48:35 +0300
Message-ID: <1de9ac6e6ed9ee827c2ecb07c51b9eda95547804.1633092363.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1633092363.git.imeevma@gmail.com>

This patch makes OP_FunctionByName, OP_AggStep and OP_BuiltinFunction to
use register P1 for the number of arguments instead of register P5. This
makes it easier to use these opcodes.

Needed for #4145
---
 src/box/sql/expr.c    |  5 ++--
 src/box/sql/select.c  |  3 +-
 src/box/sql/vdbe.c    | 64 ++++++++++++++++++++-----------------------
 src/box/sql/vdbeInt.h |  1 -
 4 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ee21c1ede..3b5df5292 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4107,18 +4107,17 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 						  (char *)coll, P4_COLLSEQ);
 			}
 			if (func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
-				sqlVdbeAddOp4(v, OP_BuiltinFunction0, constMask,
+				sqlVdbeAddOp4(v, OP_BuiltinFunction0, nFarg,
 					      r1, target, (char *)func,
 					      P4_FUNC);
 			} else {
-				sqlVdbeAddOp4(v, OP_FunctionByName, constMask,
+				sqlVdbeAddOp4(v, OP_FunctionByName, nFarg,
 					      r1, target,
 					      sqlDbStrNDup(pParse->db,
 							   func->def->name,
 							   func->def->name_len),
 					      P4_DYNAMIC);
 			}
-			sqlVdbeChangeP5(v, (u8) nFarg);
 			if (nFarg && constMask == 0) {
 				sqlReleaseTempRange(pParse, r1, nFarg);
 			}
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 2fe38e319..92e40aef6 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -5639,9 +5639,8 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo)
 			sqlVdbeAddOp4(v, OP_CollSeq, regHit, 0, 0,
 					  (char *)coll, P4_COLLSEQ);
 		}
-		sqlVdbeAddOp3(v, OP_AggStep0, 0, regAgg, pF->iMem);
+		sqlVdbeAddOp3(v, OP_AggStep0, nArg, regAgg, pF->iMem);
 		sqlVdbeAppendP4(v, pF->func, P4_FUNC);
-		sqlVdbeChangeP5(v, (u8) nArg);
 		sql_expr_type_cache_change(pParse, regAgg, nArg);
 		sqlReleaseTempRange(pParse, regAgg, nArg);
 		if (addrNext) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 86550541b..af97b0499 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1182,32 +1182,24 @@ case OP_CollSeq: {
 	break;
 }
 
-/* Opcode: BuiltinFunction0 P1 P2 P3 P4 P5
- * Synopsis: r[P3]=func(r[P2@P5])
+/* Opcode: BuiltinFunction0 P1 P2 P3 P4 *
+ * Synopsis: r[P3]=func(r[P2@P1])
  *
  * Invoke a user function (P4 is a pointer to a FuncDef object that
- * defines the function) with P5 arguments taken from register P2 and
+ * defines the function) with P1 arguments taken from register P2 and
  * successors.  The result of the function is stored in register P3.
  * Register P3 must not be one of the function inputs.
  *
- * P1 is a 32-bit bitmask indicating whether or not each argument to the
- * function was determined to be constant at compile time. If the first
- * argument was constant then bit 0 of P1 is set.
- *
  * See also: BuiltinFunction, AggStep, AggFinal
  */
-/* Opcode: BuiltinFunction P1 P2 P3 P4 P5
- * Synopsis: r[P3]=func(r[P2@P5])
+/* Opcode: BuiltinFunction P1 P2 P3 P4 *
+ * Synopsis: r[P3]=func(r[P2@P1])
  *
  * Invoke a user function (P4 is a pointer to an sql_context object that
- * contains a pointer to the function to be run) with P5 arguments taken
+ * contains a pointer to the function to be run) with P1 arguments taken
  * from register P2 and successors.  The result of the function is stored
  * in register P3.  Register P3 must not be one of the function inputs.
  *
- * P1 is a 32-bit bitmask indicating whether or not each argument to the
- * function was determined to be constant at compile time. If the first
- * argument was constant then bit 0 of P1 is set.
- *
  * SQL functions are initially coded as OP_BuiltinFunction0 with
  * P4 pointing to a FuncDef object.  But on first evaluation,
  * the P4 operand is automatically converted into an sql_context
@@ -1223,7 +1215,7 @@ case OP_BuiltinFunction0: {
 	sql_context *pCtx;
 
 	assert(pOp->p4type == P4_FUNC);
-	n = pOp->p5;
+	n = pOp->p1;
 	assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor));
 	assert(n==0 || (pOp->p2>0 && pOp->p2+n<=(p->nMem+1 - p->nCursor)+1));
 	assert(pOp->p3<pOp->p2 || pOp->p3>=pOp->p2+n);
@@ -1233,7 +1225,6 @@ case OP_BuiltinFunction0: {
 	pCtx->func = pOp->p4.func;
 	pCtx->iOp = (int)(pOp - aOp);
 	pCtx->pVdbe = p;
-	pCtx->argc = n;
 	pOp->p4type = P4_FUNCCTX;
 	pOp->p4.pCtx = pCtx;
 	pOp->opcode = OP_BuiltinFunction;
@@ -1242,6 +1233,7 @@ case OP_BuiltinFunction0: {
 }
 case OP_BuiltinFunction: {
 	int i;
+	int argc = pOp->p1;
 	sql_context *pCtx;
 
 	assert(pOp->p4type==P4_FUNCCTX);
@@ -1255,11 +1247,12 @@ case OP_BuiltinFunction: {
 	pOut = vdbe_prepare_null_out(p, pOp->p3);
 	if (pCtx->pOut != pOut) {
 		pCtx->pOut = pOut;
-		for(i=pCtx->argc-1; i>=0; i--) pCtx->argv[i] = &aMem[pOp->p2+i];
+		for(i = 0; i < argc; ++i)
+			pCtx->argv[i] = &aMem[pOp->p2 + i];
 	}
 
 #ifdef SQL_DEBUG
-	for(i=0; i<pCtx->argc; i++) {
+	for(i = 0; i < argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
 		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
@@ -1267,7 +1260,7 @@ case OP_BuiltinFunction: {
 	pCtx->is_aborted = false;
 	assert(pCtx->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
 	struct func_sql_builtin *func = (struct func_sql_builtin *)pCtx->func;
-	func->call(pCtx, pCtx->argc, pCtx->argv);
+	func->call(pCtx, argc, pCtx->argv);
 
 	/* If the function returned an error, throw an exception */
 	if (pCtx->is_aborted)
@@ -1283,11 +1276,11 @@ case OP_BuiltinFunction: {
 	break;
 }
 
-/* Opcode: FunctionByName * P2 P3 P4 P5
- * Synopsis: r[P3]=func(r[P2@P5])
+/* Opcode: FunctionByName P1 P2 P3 P4 *
+ * Synopsis: r[P3]=func(r[P2@P1])
  *
  * Invoke a user function (P4 is a pointer to a function object
- * that defines the function) with P5 arguments taken from
+ * that defines the function) with P1 arguments taken from
  * register P2 and successors. The result of the function is
  * stored in register P3.
  */
@@ -1303,7 +1296,7 @@ case OP_FunctionByName: {
 	 * turn out to be invalid after call.
 	 */
 	enum field_type returns = func->def->returns;
-	int argc = pOp->p5;
+	int argc = pOp->p1;
 	struct Mem *argv = &aMem[pOp->p2];
 	struct port args, ret;
 
@@ -4177,26 +4170,26 @@ case OP_DecrJumpZero: {      /* jump, in1 */
 }
 
 
-/* Opcode: AggStep0 * P2 P3 P4 P5
- * Synopsis: accum=r[P3] step(r[P2@P5])
+/* Opcode: AggStep0 P1 P2 P3 P4 *
+ * Synopsis: accum=r[P3] step(r[P2@P1])
  *
  * Execute the step function for an aggregate.  The
- * function has P5 arguments.   P4 is a pointer to the FuncDef
+ * function has P1 arguments.   P4 is a pointer to the FuncDef
  * structure that specifies the function.  Register P3 is the
  * accumulator.
  *
- * The P5 arguments are taken from register P2 and its
+ * The P1 arguments are taken from register P2 and its
  * successors.
  */
-/* Opcode: AggStep * P2 P3 P4 P5
- * Synopsis: accum=r[P3] step(r[P2@P5])
+/* Opcode: AggStep P1 P2 P3 P4 *
+ * Synopsis: accum=r[P3] step(r[P2@P1])
  *
  * Execute the step function for an aggregate.  The
- * function has P5 arguments.   P4 is a pointer to an sql_context
+ * function has P1 arguments.   P4 is a pointer to an sql_context
  * object that is used to run the function.  Register P3 is
  * as the accumulator.
  *
- * The P5 arguments are taken from register P2 and its
+ * The P1 arguments are taken from register P2 and its
  * successors.
  *
  * This opcode is initially coded as OP_AggStep0.  On first evaluation,
@@ -4220,7 +4213,6 @@ case OP_AggStep0: {
 	pCtx->func = pOp->p4.func;
 	pCtx->iOp = (int)(pOp - aOp);
 	pCtx->pVdbe = p;
-	pCtx->argc = n;
 	pOp->p4type = P4_FUNCCTX;
 	pOp->p4.pCtx = pCtx;
 	pOp->opcode = OP_AggStep;
@@ -4229,6 +4221,7 @@ case OP_AggStep0: {
 }
 case OP_AggStep: {
 	int i;
+	int argc = pOp->p1;
 	sql_context *pCtx;
 	Mem *pMem;
 	Mem t;
@@ -4244,11 +4237,12 @@ case OP_AggStep: {
 	 */
 	if (pCtx->pMem != pMem) {
 		pCtx->pMem = pMem;
-		for(i=pCtx->argc-1; i>=0; i--) pCtx->argv[i] = &aMem[pOp->p2+i];
+		for(i = 0; i < argc; ++i)
+			pCtx->argv[i] = &aMem[pOp->p2 + i];
 	}
 
 #ifdef SQL_DEBUG
-	for(i=0; i<pCtx->argc; i++) {
+	for(i = 0; i < argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
 		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
@@ -4261,7 +4255,7 @@ case OP_AggStep: {
 	pCtx->skipFlag = 0;
 	assert(pCtx->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
 	struct func_sql_builtin *func = (struct func_sql_builtin *)pCtx->func;
-	func->call(pCtx, pCtx->argc, pCtx->argv);
+	func->call(pCtx, argc, pCtx->argv);
 	if (pCtx->is_aborted) {
 		mem_destroy(&t);
 		goto abort_due_to_error;
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index cfe743b94..575ab3f3d 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -182,7 +182,6 @@ struct sql_context {
 	 */
 	bool is_aborted;
 	u8 skipFlag;		/* Skip accumulator loading if true */
-	u8 argc;		/* Number of arguments */
 	sql_value *argv[1];	/* Argument set */
 };
 
-- 
2.25.1


  parent reply	other threads:[~2021-10-01 12:50 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 12:48 [Tarantool-patches] [PATCH v4 00/16] sql: refactor aggregate functions Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 01/16] sql: remove MEM_Zero flag from struct MEM Mergen Imeev via Tarantool-patches
2021-10-04 21:51   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-05  8:46     ` Mergen Imeev via Tarantool-patches
2021-10-05  9:42       ` Mergen Imeev via Tarantool-patches
2021-10-05 12:28         ` Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 02/16] sql: fix possible undefined behavior during cast Mergen Imeev via Tarantool-patches
2021-10-04 21:52   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-05  9:00     ` Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` Mergen Imeev via Tarantool-patches [this message]
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 04/16] sql: remove AggStep0 and OP_BuiltinFunction0 Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 05/16] sql: move collation to struct sql_context Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 06/16] sql: introduce mem_append() Mergen Imeev via Tarantool-patches
2021-10-04 21:52   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-05  9:32     ` Mergen Imeev via Tarantool-patches
2021-10-11 21:50       ` Vladislav Shpilevoy via Tarantool-patches
2021-10-19 10:49         ` Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 07/16] sql: remove sql_vdbemem_finalize() Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 08/16] sql: refactor SUM() function Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 09/16] sql: refactor TOTAL() function Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 10/16] sql: refactor AVG() function Mergen Imeev via Tarantool-patches
2021-10-04 21:53   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-05  9:48     ` Mergen Imeev via Tarantool-patches
2021-10-11 21:50       ` Vladislav Shpilevoy via Tarantool-patches
2021-10-19 11:14         ` Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 11/16] sql: refactor COUNT() function Mergen Imeev via Tarantool-patches
2021-10-04 21:53   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-05  9:55     ` Mergen Imeev via Tarantool-patches
2021-10-11 21:51       ` Vladislav Shpilevoy via Tarantool-patches
2021-10-19 11:17         ` Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 12/16] sql: refactor MIN() and MAX() functions Mergen Imeev via Tarantool-patches
2021-10-04 21:54   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-05 10:07     ` Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 13/16] sql: refactor GROUP_CONCAT() function Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 14/16] sql: remove copying of result in finalizers Mergen Imeev via Tarantool-patches
2021-10-01 12:48 ` [Tarantool-patches] [PATCH v4 15/16] sql: remove MEM_TYPE_AGG Mergen Imeev via Tarantool-patches
2021-10-01 12:49 ` [Tarantool-patches] [PATCH v4 16/16] sql: remove field argv from struct sql_context Mergen Imeev via Tarantool-patches
2021-10-25 20:58 ` [Tarantool-patches] [PATCH v4 00/16] sql: refactor aggregate functions Vladislav Shpilevoy via Tarantool-patches
2021-10-26 10:34 Mergen Imeev via Tarantool-patches
2021-10-26 10:34 ` [Tarantool-patches] [PATCH v4 03/16] sql: use register P1 for number of arguments 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=1de9ac6e6ed9ee827c2ecb07c51b9eda95547804.1633092363.git.imeevma@gmail.com \
    --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