From de9a7b1a37ec34054b062c513fa1bf9357699a8d Mon Sep 17 00:00:00 2001
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Date: Mon, 16 Sep 2019 15:36:26 +0300
Subject: [PATCH] sql: use name instead of function pointer for UDF

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   | 25 +++++++++++++++++++++++++
 test/sql/checks.test.lua | 10 ++++++++++
 4 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 6d5ad2a272..648b7170ee 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 f50198281f..9495ada37f 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@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 6d584c3013..a952b2b70b 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -839,6 +839,31 @@ 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
+...
+box.func.MYFUNC:drop()
+---
+...
+box.execute("INSERT INTO t6 VALUES(11);");
+---
+- null
+- Function 'MYFUNC' does not exist
+...
+box.execute("DROP TABLE t6")
+---
+- row_count: 1
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index f8dc5d3638..4d33823536 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -272,4 +272,14 @@ 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);");
+box.func.MYFUNC:drop()
+box.execute("INSERT INTO t6 VALUES(11);");
+box.execute("DROP TABLE t6")
+
 test_run:cmd("clear filter")
-- 
GitLab