Fix the remaining issues about default keyword in proc/func call (bab…

…elfish-for-postgresql#2039) 1. The error message should be the same as sql server 2. Function call with not exist default param, then should use NULL value 3. Support call by name default as well Task: BABEL-335 Signed-off-by: Zhibai Song <[email protected]>
amazon-aurora · Dec 24, 2023 · 89206e9 · 89206e9
1 parent e9db869
commit 89206e9
Show file tree

Hide file tree

Showing 10 changed files with 223 additions and 23 deletions.
diff --git a/contrib/babelfishpg_tds/error_mapping.txt b/contrib/babelfishpg_tds/error_mapping.txt
@@ -81,6 +81,7 @@ XX000 ERRCODE_INTERNAL_ERROR	"CREATE FUNCTION failed because a column name is no
 42883 ERRCODE_UNDEFINED_FUNCTION	"could not identify an equality operator for type %s"	SQL_ERROR_306	16
 42883 ERRCODE_UNDEFINED_FUNCTION	"could not identify an ordering operator for type %s"	SQL_ERROR_306	16
 42883 ERRCODE_UNDEFINED_FUNCTION	"%s %s expects parameter \"%s\", which was not supplied."	SQL_ERROR_201	16
+42883 ERRCODE_UNDEFINED_FUNCTION	"Procedure or function \'%s\' expects parameter \'%s\', which was not supplied."	SQL_ERROR_201	16
 42883 ERRCODE_UNDEFINED_FUNCTION	"%s %s has no parameters and arguments were supplied."	SQL_ERROR_8146	16
 42883 ERRCODE_UNDEFINED_FUNCTION	"%s %s has too many arguments specified."	SQL_ERROR_8144	16
 42883 ERRCODE_UNDEFINED_FUNCTION	"\"%s\" is not an parameter for %s %s."	SQL_ERROR_8145	16

diff --git a/contrib/babelfishpg_tsql/src/hooks.c b/contrib/babelfishpg_tsql/src/hooks.c
@@ -94,6 +94,7 @@ static bool match_pltsql_func_call(HeapTuple proctup, int nargs, List *argnames,
 static ObjectAddress get_trigger_object_address(List *object, Relation *relp, bool missing_ok, bool object_from_input);
 Oid			get_tsql_trigger_oid(List *object, const char *tsql_trigger_name, bool object_from_input);
 static Node *transform_like_in_add_constraint(Node *node);
+static char** fetch_func_input_arg_names(HeapTuple func_tuple);
 
 /*****************************************
  * 			Analyzer Hooks
@@ -3559,25 +3560,57 @@ PlTsqlMatchNamedCall(HeapTuple proctup, int nargs, List *argnames,
 
 static int getDefaultPosition(const List *default_positions, const ListCell *def_idx, int argPosition)
 {
-	int currPosition = intVal((Node *) lfirst(def_idx));
+	int currPosition;
+	if (default_positions == NIL || def_idx == NULL)
+		return -1;
+	currPosition = intVal((Node *) lfirst(def_idx));
 	while (currPosition != argPosition)
 	{
 		def_idx = lnext(default_positions, def_idx);
 		if (def_idx == NULL)
 		{
-			elog(ERROR, "not enough default arguments");
 			return -1;
 		}
 		currPosition = intVal((Node *) lfirst(def_idx));
 	}
 	return list_cell_number(default_positions, def_idx);
 }
 
+/**
+ * @brief fetch the func input arg names
+ * 
+ * @param func_tuple or proc_tuple
+ * @return char** list of input arg names
+ */
+static char** fetch_func_input_arg_names(HeapTuple func_tuple)
+{
+	Datum proargnames;
+	Datum		proargmodes;
+	char**		arg_names;
+	bool 		isnull;
+
+	proargnames = SysCacheGetAttr(PROCNAMEARGSNSP, func_tuple,
+					Anum_pg_proc_proargnames,
+					&isnull);
+
+	proargmodes = SysCacheGetAttr(PROCNAMEARGSNSP, func_tuple,
+					Anum_pg_proc_proargmodes,
+					&isnull);
+
+	if (isnull)
+		proargmodes = PointerGetDatum(NULL);	/* just to be sure */
+
+	get_func_input_arg_names(proargnames,
+									proargmodes,
+									&arg_names);
+	return arg_names;
+}
+
 /**
  * @brief farg position default should get the corresponding default position value
  * 
  * @param func_tuple 
- * @param defaults 
+ * @param defaults can be NIL
  * @param fargs 
  * @return List* 
  */
@@ -3586,19 +3619,24 @@ replace_pltsql_function_defaults(HeapTuple func_tuple, List *defaults, List *far
 
 {
 	HeapTuple	bbffunctuple;
+	Form_pg_proc proc_form;
 
 	if (sql_dialect != SQL_DIALECT_TSQL)
 		return fargs;
 
 	bbffunctuple = get_bbf_function_tuple_from_proctuple(func_tuple);
+	proc_form = (Form_pg_proc) GETSTRUCT(func_tuple);
 
-	if (HeapTupleIsValid(bbffunctuple)){
+	if (HeapTupleIsValid(bbffunctuple))
+	{
 		Datum		arg_default_positions;
 		bool		isnull;
 		char	   *str;
 		List	   *default_positions = NIL, *ret = NIL;
 		ListCell   *def_idx;
 		ListCell   *lc;
+		char	  **arg_names;
+
 		int		   position,i,j;
 
 		/* Fetch default positions */
@@ -3607,46 +3645,86 @@ replace_pltsql_function_defaults(HeapTuple func_tuple, List *defaults, List *far
 												Anum_bbf_function_ext_default_positions,
 												&isnull);
 
-		if (isnull)
-			elog(ERROR, "not enough default arguments");
-
-		str =TextDatumGetCString(arg_default_positions);
-		default_positions = castNode(List, stringToNode(str));
-		pfree(str);
+		if (!isnull)
+		{
+			str =TextDatumGetCString(arg_default_positions);
+			default_positions = castNode(List, stringToNode(str));
+			pfree(str);
 
-		def_idx = list_head(default_positions);
+			def_idx = list_head(default_positions);
+		}
+		else
+		{
+			default_positions = NIL;
+			def_idx = NULL;
+		}
 		i = 0;
 
 		foreach(lc, fargs)
 		{
+			bool has_default = false;
 			if (nodeTag((Node*)lfirst(lc)) == T_RelabelType &&
 				nodeTag(((RelabelType*)lfirst(lc))->arg) == T_SetToDefault)
 			{
 				position = getDefaultPosition(default_positions, def_idx, i);
-				ret = lappend(ret, list_nth(defaults, position));
+				if (position >= 0)
+				{
+					ret = lappend(ret, list_nth(defaults, position));
+					has_default = true;
+				}
+				else if (proc_form->prokind == PROKIND_FUNCTION)
+				{
+					ret = lappend(ret, makeNullConst(proc_form->proargtypes.values[i], -1, InvalidOid));
+					has_default = true;
+				}
 			}
-			else if (nodeTag((Node*)lfirst(lc)) == T_FuncExpr)
-			{
-				if(((FuncExpr*)lfirst(lc))->funcformat == COERCE_IMPLICIT_CAST &&
+			else if (nodeTag((Node*)lfirst(lc)) == T_FuncExpr && 
+			((FuncExpr*)lfirst(lc))->funcformat == COERCE_IMPLICIT_CAST &&
 					nodeTag(linitial(((FuncExpr*)lfirst(lc))->args)) == T_SetToDefault)
-				{
+			{
 				// We'll keep the implicit cast function when it needs implicit cast
-					FuncExpr *funcExpr = (FuncExpr*)lfirst(lc);
-					List *newArgs = NIL;
-					position = getDefaultPosition(default_positions, def_idx, i);
+				FuncExpr *funcExpr = (FuncExpr*)lfirst(lc);
+				List *newArgs = NIL;
+				position = getDefaultPosition(default_positions, def_idx, i);
+				if (position >= 0)
+				{
 					newArgs = lappend(newArgs, list_nth(defaults, position));
 					for (j = 1; j < list_length(funcExpr->args); ++j)
 						newArgs = lappend(newArgs, list_nth(funcExpr->args, j));
 					funcExpr->args = newArgs;
 					ret = lappend(ret, funcExpr);
+					has_default = true;
+				}
+				else if (proc_form->prokind == PROKIND_FUNCTION)
+				{
+					newArgs = lappend(newArgs, makeNullConst(proc_form->proargtypes.values[i], -1, InvalidOid));
+					for (j = 1; j < list_length(funcExpr->args); ++j)
+						newArgs = lappend(newArgs, list_nth(funcExpr->args, j));
+					funcExpr->args = newArgs;
+					ret = lappend(ret, funcExpr);
+					has_default = true;
 				}
 			}
-			else ret = lappend(ret, lfirst(lc));
+			else 
+			{
+				ret = lappend(ret, lfirst(lc));	
+				has_default = true;
+			}
+			if (!has_default)
+			{
+				arg_names = fetch_func_input_arg_names(func_tuple);
+
+				if (proc_form->prokind == PROKIND_PROCEDURE)
+					ereport(ERROR,
+						(errcode(ERRCODE_UNDEFINED_FUNCTION),
+						errmsg("Procedure or function \'%s\' expects parameter \'%s\', which was not supplied.",
+							NameStr(proc_form->proname), arg_names[i])));
+			}
 			++i;
 		}
-		return ret;
-
 		ReleaseSysCache(bbffunctuple);
+
+		return ret;
 	}
 	else
 	{

diff --git a/test/JDBC/expected/TestErrorHelperFunctions.out b/test/JDBC/expected/TestErrorHelperFunctions.out
@@ -104,6 +104,7 @@ XX000#!#CREATE FUNCTION failed because a column name is not specified for column
 42883#!#could not identify an equality operator for type %s#!##!#306
 42883#!#could not identify an ordering operator for type %s#!##!#306
 42883#!#%s %s expects parameter "%s", which was not supplied.#!##!#201
+42883#!#Procedure or function '%s' expects parameter '%s', which was not supplied.#!##!#201
 42883#!#%s %s has no parameters and arguments were supplied.#!##!#8146
 42883#!#%s %s has too many arguments specified.#!##!#8144
 42883#!#"%s" is not an parameter for %s %s.#!##!#8145

diff --git a/test/JDBC/expected/TestErrorHelperFunctionsUpgrade-vu-verify.out b/test/JDBC/expected/TestErrorHelperFunctionsUpgrade-vu-verify.out
@@ -211,6 +211,7 @@ int
 306
 306
 201
+201
 8146
 8144
 8145
@@ -381,6 +382,7 @@ int
 306
 306
 201
+201
 8146
 8144
 8145
@@ -490,6 +492,6 @@ EXEC TestErrorHelperFunctionsUpgrade_VU_PREPARE_PROC
 GO
 ~~START~~
 int
-163
+164
 ~~END~~
 
diff --git a/test/JDBC/expected/default_params-vu-cleanup.out b/test/JDBC/expected/default_params-vu-cleanup.out
@@ -7,6 +7,9 @@ GO
 drop function default_params_func3;
 GO
 
+drop function default_params_func4;
+GO
+
 drop procedure default_params_proc1;
 GO
 
@@ -18,3 +21,6 @@ GO
 
 drop procedure default_params_proc4
 GO
+
+drop function default_params_func5;
+GO
diff --git a/test/JDBC/expected/default_params-vu-prepare.out b/test/JDBC/expected/default_params-vu-prepare.out
@@ -7,6 +7,12 @@ GO
 create function default_params_func3 (@p1 varchar(20) = 'abc') returns varchar as begin return @p1 end;
 GO
 
+create function default_params_func4 (@p1 int) returns int as begin return @p1 end;
+GO
+
+create function default_params_func5 (@p1 varchar(20)) returns varchar as begin return @p1 end;
+GO
+
 create proc default_params_proc1 @p1 int=1, @p2 int=2, @p3 int=3 as select @p1, @p2, @p3
 GO
 

diff --git a/test/JDBC/expected/default_params-vu-verify.out b/test/JDBC/expected/default_params-vu-verify.out
@@ -78,6 +78,23 @@ d
 ~~END~~
 
 
+-- it'll use default 
+select default_params_func4(default);
+GO
+~~START~~
+int
+<NULL>
+~~END~~
+
+
+select default_params_func5(default);
+GO
+~~START~~
+varchar
+<NULL>
+~~END~~
+
+
 exec default_params_proc1 111, default, 333
 GO
 ~~START~~
@@ -102,6 +119,30 @@ int#!#int#!#int
 ~~END~~
 
 
+exec default_params_proc1 @p1=default, @p2=default,@p3=300
+GO
+~~START~~
+int#!#int#!#int
+1#!#2#!#300
+~~END~~
+
+
+exec default_params_proc1 @p1=300, @p2=default,@p3=default
+GO
+~~START~~
+int#!#int#!#int
+300#!#2#!#3
+~~END~~
+
+
+exec default_params_proc1 @p1=default, @p2=300,@p3=default
+GO
+~~START~~
+int#!#int#!#int
+1#!#300#!#3
+~~END~~
+
+
 exec default_params_proc2 default, 2
 GO
 ~~START~~
@@ -134,10 +175,35 @@ d#!#3
 ~~END~~
 
 
+-- verify the error message
+exec default_params_proc3 default, default
+GO
+~~ERROR (Code: 201)~~
+
+~~ERROR (Message: Procedure or function 'default_params_proc3' expects parameter '@p2', which was not supplied.)~~
+
+
+-- verify the error message
+exec default_params_proc3 'ddd', default
+GO
+~~ERROR (Code: 201)~~
+
+~~ERROR (Message: Procedure or function 'default_params_proc3' expects parameter '@p2', which was not supplied.)~~
+
+
+-- verify the type cast
 exec default_params_proc4 1,2,default
 GO
 ~~START~~
 int#!#int#!#varchar
 1#!#2#!#dbb
 ~~END~~
 
+
+exec default_params_proc4 1,2, @p3=default
+GO
+~~START~~
+int#!#int#!#varchar
+1#!#2#!#dbb
+~~END~~
+
diff --git a/test/JDBC/input/default_params-vu-cleanup.sql b/test/JDBC/input/default_params-vu-cleanup.sql
@@ -7,6 +7,9 @@ GO
 drop function default_params_func3;
 GO
 
+drop function default_params_func4;
+GO
+
 drop procedure default_params_proc1;
 GO
 
@@ -17,4 +20,7 @@ drop procedure default_params_proc3
 GO
 
 drop procedure default_params_proc4
+GO
+
+drop function default_params_func5;
 GO
diff --git a/test/JDBC/input/default_params-vu-prepare.sql b/test/JDBC/input/default_params-vu-prepare.sql
@@ -7,6 +7,12 @@ GO
 create function default_params_func3 (@p1 varchar(20) = 'abc') returns varchar as begin return @p1 end;
 GO
 
+create function default_params_func4 (@p1 int) returns int as begin return @p1 end;
+GO
+
+create function default_params_func5 (@p1 varchar(20)) returns varchar as begin return @p1 end;
+GO
+
 create proc default_params_proc1 @p1 int=1, @p2 int=2, @p3 int=3 as select @p1, @p2, @p3
 GO
-Original file line number
+Diff line change
@@ Expand Up / @@ -211,6 +211,7 @@ int @@
@@ Expand Down Expand Up / @@ -381,6 +382,7 @@ int @@
@@ Expand Down Expand Up / @@ -490,6 +492,6 @@ EXEC TestErrorHelperFunctionsUpgrade_VU_PREPARE_PROC @@
     GO
     ~~START~~
     int
     ~~END~~