From 4688179c11badff80a603600908096e1b333a760 Mon Sep 17 00:00:00 2001
From: Adrian Grucza <adrian.grucza@iress.com>
Date: Mon, 10 Jan 2022 12:24:00 +1100
Subject: [PATCH 1/2] Fix FetchRefcursors issues 1. Fix crash if error occurs
 when fetching noninitial refcursor 2. Fix error when fetching NULL refcursor
 3. Return empty result if all refcursors are NULL

---
 statement.c                        | 20 +++++++++++++++-----
 test/expected/fetch-refcursors.out | 36 ++++++++++++++++++++++++++++++++----
 test/src/fetch-refcursors-test.c   | 29 +++++++++++++++++++----------
 3 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/statement.c b/statement.c
index e7f816f..f45cd79 100644
--- a/statement.c
+++ b/statement.c
@@ -2304,6 +2304,7 @@ MYLOG(DETAIL_LOG_LEVEL, "!!SC_fetch return =%d\n", ret);
 		{
 			char			fetch[128];
 			QResultClass 	*last = NULL, *res;
+			BOOL			refcursor_found = FALSE;
 
 			/* Iterate the columns in the result to look for refcursors */
 			numcols = QR_NumResultCols(rhold.first);
@@ -2318,7 +2319,12 @@ MYLOG(DETAIL_LOG_LEVEL, "!!SC_fetch return =%d\n", ret);
 						break;
 					}
 
+					refcursor_found = TRUE;
 					STR_TO_NAME(self->cursor_name, QR_get_value_backend_text(rhold.first, 0, i));
+					/* Skip NULL refcursors (allows procedure to return a variable number of results) */
+					if (!SC_cursor_is_valid(self))
+						continue;
+
 					SC_set_fetchcursor(self);
 					qi.result_in = NULL;
 					qi.cursor = SC_cursor_name(self);
@@ -2339,19 +2345,23 @@ MYLOG(DETAIL_LOG_LEVEL, "!!SC_fetch return =%d\n", ret);
 							QR_concat(last, res);
 							self->multi_statement = TRUE;
 						}
+						last = res;
 						if (!QR_command_maybe_successful(res))
 						{
 							SC_set_errorinfo(self, res, 0);
-							QR_Destructor(rhold.first);
 							break;
 						}
-
-						last = res;
 					}
 				}
 			}
-			if (last)
-				QR_Destructor(rhold.first);
+			if (refcursor_found)
+			{
+				/* Discard original result */
+				if (NULL == last)
+					SC_set_Result(self, QR_Constructor());	/* return empty result */
+				else
+					QR_Destructor(rhold.first);
+			}
 		}
 	}
 cleanup:
diff --git a/test/expected/fetch-refcursors.out b/test/expected/fetch-refcursors.out
index fff9ee1..bfe6f85 100644
--- a/test/expected/fetch-refcursors.out
+++ b/test/expected/fetch-refcursors.out
@@ -2,19 +2,47 @@ Creating procedure 'refproc'
 connected
 disconnecting
 
--- TEST using FetchRefcursors=0 and SQL_ATTR_AUTOCOMMIT=1
+-- TEST using FetchRefcursors=0, autocommit=1, numresults=2
 connected
 Output param num_cursor is 2
 --1 Result set:
-2	ref1	ref2
+2	<unnamed portal 1>	<unnamed portal 2>
 disconnecting
 
--- TEST using FetchRefcursors=1 and SQL_ATTR_AUTOCOMMIT=1
+-- TEST using FetchRefcursors=1, autocommit=1, numresults=2
 connected
 SQLExecute failed
 HY000=Query must be executed in a transaction when FetchRefcursors setting is enabled.
 
--- TEST using FetchRefcursors=1 and SQL_ATTR_AUTOCOMMIT=0
+-- TEST using FetchRefcursors=1, autocommit=0, numresults=0
+connected
+Output param num_cursor is 0
+--1 Result set:
+disconnecting
+
+-- TEST using FetchRefcursors=1, autocommit=0, numresults=1
+connected
+Output param num_cursor is 1
+--1 Result set:
+1	foo
+2	bar
+3	foobar
+disconnecting
+
+-- TEST using FetchRefcursors=1, autocommit=0, numresults=2
+connected
+Output param num_cursor is 2
+--1 Result set:
+1	foo
+2	bar
+3	foobar
+--2 Result set:
+foobar	3
+bar	2
+foo	1
+disconnecting
+
+-- TEST using FetchRefcursors=1, autocommit=0, numresults=3
 connected
 Output param num_cursor is 2
 --1 Result set:
diff --git a/test/src/fetch-refcursors-test.c b/test/src/fetch-refcursors-test.c
index ed9a47a..01e58e4 100644
--- a/test/src/fetch-refcursors-test.c
+++ b/test/src/fetch-refcursors-test.c
@@ -33,13 +33,19 @@ static void setup_procedure()
 	CHECK_CONN_RESULT(rc, "failed to allocate stmt handle", conn);
 
 	rc = SQLExecDirect(hstmt, "create or replace procedure refproc"
-			"(inout num_cursor integer, inout ref1 refcursor default 'ref1', inout ref2 refcursor default 'ref2') as "
+			"(inout num_cursor integer, inout ref1 refcursor default null, inout ref2 refcursor default null) as "
 			"$procedure$ \n"
 			"DECLARE \n"
 			"BEGIN \n"
-			"num_cursor := 2; \n"
-			"OPEN ref1 FOR SELECT id, t FROM testtab1 ORDER BY id ASC; \n"
-			"OPEN ref2 FOR SELECT t, id FROM testtab1 ORDER BY id DESC; \n"
+			"IF num_cursor > 0 THEN \n"
+			"    OPEN ref1 FOR SELECT id, t FROM testtab1 ORDER BY id ASC; \n"
+			"END IF; \n"
+			"IF num_cursor > 1 THEN \n"
+			"    OPEN ref2 FOR SELECT t, id FROM testtab1 ORDER BY id DESC; \n"
+			"END IF; \n"
+			"IF num_cursor > 2 THEN \n"
+			"    num_cursor := 2; \n"
+			"END IF; \n"
 			"END; \n"
 			"$procedure$ \n"
 			"LANGUAGE plpgsql\n"
@@ -52,13 +58,13 @@ static void setup_procedure()
 	test_disconnect();
 }
 
-static void refcursor_test(char* connectparams, SQLUINTEGER autocommit)
+static void refcursor_test(char* connectparams, SQLUINTEGER autocommit, int numresults)
 {
 	SQLRETURN	rc;
 	HSTMT		hstmt = SQL_NULL_HSTMT;
-	int	        num_cursor = 0;
+	int	        num_cursor = numresults;
 
-	printf("\n-- TEST using %s and SQL_ATTR_AUTOCOMMIT=%u\n", connectparams, autocommit);
+	printf("\n-- TEST using %s, autocommit=%u, numresults=%d\n", connectparams, autocommit, numresults);
 
 	test_connect_ext(connectparams);
 
@@ -102,9 +108,12 @@ int main(int argc, char **argv)
 {
 	setup_procedure();
 
-	refcursor_test("FetchRefcursors=0", SQL_AUTOCOMMIT_ON);
-	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_ON);
-	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_OFF);
+	refcursor_test("FetchRefcursors=0", SQL_AUTOCOMMIT_ON, 2);
+	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_ON, 2);
+	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_OFF, 0);
+	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_OFF, 1);
+	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_OFF, 2);
+	refcursor_test("FetchRefcursors=1", SQL_AUTOCOMMIT_OFF, 3);
 
 	return 0;
 }
-- 
2.13.0.windows.1

