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

[Kirill Shcherbatov]

> Could you add test case covering mentioned situation?
Yes. Appended.

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

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.

Closes #4176
---
 src/box/sql/expr.c       | 17 ++++++++++++-----
 src/box/sql/vdbe.c       | 11 ++++++++---
 test/sql/checks.result   | 28 ++++++++++++++++++++++++++++
 test/sql/checks.test.lua | 11 +++++++++++
 4 files changed, 59 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;
diff --git a/test/sql/checks.result b/test/sql/checks.result
index b8bd19a84..02ee82a4b 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -918,6 +918,34 @@ box.execute("DROP TABLE test;")
 ---
 - row_count: 1
 ...
+--
+-- gh-4176: Can't recover if check constraint involves function.
+--
+function myfunc(x) return x < 10 end
+---
+...
+box.schema.func.create('MYFUNC', {param_list = {'integer'}, returns = 'integer', is_deterministic = true, exports = {'LUA', 'SQL'}})
+---
+...
+box.execute("CREATE TABLE t6(a  INT CHECK (myfunc(a)) primary key);");
+---
+- row_count: 1
+...
+myfunc = nil
+---
+...
+box.execute("INSERT INTO t6 VALUES(11);");
+---
+- null
+- Procedure 'MYFUNC' is not defined
+...
+box.execute("DROP TABLE t6")
+---
+- row_count: 1
+...
+box.func.MYFUNC:drop()
+---
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index 832322190..fa68920f6 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -302,4 +302,15 @@ assert(box.space.TEST.ck_constraint.some_ck.is_enabled == true)
 box.space.TEST:insert({12})
 box.execute("DROP TABLE test;")
 
+--
+-- gh-4176: Can't recover if check constraint involves function.
+--
+function myfunc(x) return x < 10 end
+box.schema.func.create('MYFUNC', {param_list = {'integer'}, returns = 'integer', is_deterministic = true, exports = {'LUA', 'SQL'}})
+box.execute("CREATE TABLE t6(a  INT CHECK (myfunc(a)) primary key);");
+myfunc = nil
+box.execute("INSERT INTO t6 VALUES(11);");
+box.execute("DROP TABLE t6")
+box.func.MYFUNC:drop()
+
 test_run:cmd("clear filter")
-- 
2.23.0