[tarantool-patches] Re: [PATCH v3 4/4] sql: use name instead of function pointer for UDF

[Kirill Shcherbatov]

> 
> Usually we protect pointers with schema_version, rather than avoid
> them at all. Please take a look how iterators work.
> 

We've discussed this question in T. and decided that current approach is ok except the
necessity to rename OP_Function to OP_FunctionByName. Done.

===========================================

This patch changes OP_Function parameters convention: now a
function's name is passed instead of pointer to the function
object. This allows to normally handle the situation, when UDF
has been deleted to the moment of the VDBE code execution.
In particular case this may happen with CK constraints that
refers to a deleted persistent function.
---
 src/box/sql/expr.c | 17 ++++++++++++-----
 src/box/sql/vdbe.c | 11 ++++++++---
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 6d5ad2a27..648b7170e 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4136,11 +4136,18 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 				sqlVdbeAddOp4(v, OP_CollSeq, 0, 0, 0,
 						  (char *)coll, P4_COLLSEQ);
 			}
-			int op = func->def->language ==
-				 FUNC_LANGUAGE_SQL_BUILTIN ?
-				 OP_BuiltinFunction0 : OP_Function;
-			sqlVdbeAddOp4(v, op, constMask, r1, target,
-				      (char *)func, P4_FUNC);
+			if (func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
+				sqlVdbeAddOp4(v, OP_BuiltinFunction0, constMask,
+					      r1, target, (char *)func,
+					      P4_FUNC);
+			} else {
+				sqlVdbeAddOp4(v, OP_FunctionByName, constMask,
+					      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/vdbe.c b/src/box/sql/vdbe.c
index f50198281..9495ada37 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1828,7 +1828,7 @@ case OP_BuiltinFunction: {
 	break;
 }
 
-/* Opcode: Function * P2 P3 P4 P5
+/* Opcode: FunctionByName * P2 P3 P4 P5
  * Synopsis: r[P3]=func(r[P2 at P5])
  *
  * Invoke a user function (P4 is a pointer to a function object
@@ -1836,8 +1836,13 @@ case OP_BuiltinFunction: {
  * register P2 and successors. The result of the function is
  * stored in register P3.
  */
-case OP_Function: {
-	struct func *func = pOp->p4.func;
+case OP_FunctionByName: {
+	assert(pOp->p4type == P4_DYNAMIC);
+	struct func *func = func_by_name(pOp->p4.z, strlen(pOp->p4.z));
+	if (unlikely(func == NULL)) {
+		diag_set(ClientError, ER_NO_SUCH_FUNCTION, pOp->p4.z);
+		goto abort_due_to_error;
+	}
 	int argc = pOp->p5;
 	struct Mem *argv = &aMem[pOp->p2];
 	struct port args, ret;
-- 
2.23.0