Fix resetting some GUCs at the time of TDSResetConnection (#3096)

Some GUCs had an issue with being reset at time of TDSResetConnection: 1. Explain GUCs, they were not getting setting reset after TDSResetConnection. To fix this we need to reset it at time TDSResetConnection AND also at time of sys.sp_reset_connection execution(which would otherwise have given explain output). 2. ANSI_DEFAULTS and IMPLICIT_TRANSACTION related GUCs. These were getting set when driver sends some TDS flags. But after ResetConnection they were being rolled-back. To avoid this we implement TdsResetLoginFlags a wrapper to Re-ProcessLoginFlags. 3. We restrict the setting and re-setting of important GUCs like "babelfishpg_tsql.migration_mode" and "role", since once set, cannot be reset. Issues Resolved: BABEL-5280, BABEL-5276, BABEL-5278
babelfish-for-postgresql · Nov 19, 2024 · cee4e1a · cee4e1a
1 parent 5c3ff29
commit cee4e1a
Show file tree

Hide file tree

Showing 45 changed files with 1,066 additions and 45 deletions.
diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdslogin.c b/contrib/babelfishpg_tds/src/backend/tds/tdslogin.c
@@ -1142,6 +1142,17 @@ ProcessLoginFlags(LoginRequest loginInfo)
 	}
 }
 
+/*
+ * TdsResetLoginFlags - Resets the session properties which
+ * we provided during login time.
+ * Wrapper of ProcessLoginFlags, since we do not expose loginInfo.
+ */
+void
+TdsResetLoginFlags()
+{
+	ProcessLoginFlags(loginInfo);
+}
+
 /*
  * ProcessLoginInternal - internal workhorse for processing login
  * request.

diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsprotocol.c b/contrib/babelfishpg_tds/src/backend/tds/tdsprotocol.c
@@ -154,6 +154,8 @@ ResetTDSConnection(void)
 	TdsResetCache();
 	TdsResponseReset();
 	TdsResetBcpOffset();
+	TdsResetLoginFlags();
+
 	/* Retore previous isolation level when not called by sys.sp_reset_connection. */
 	if (!resetTdsConnectionFlag)
 	{

diff --git a/contrib/babelfishpg_tds/src/include/tds_int.h b/contrib/babelfishpg_tds/src/include/tds_int.h
@@ -303,6 +303,7 @@ extern void TdsSendLoginAck(Port *port);
 extern uint32_t GetClientTDSVersion(void);
 extern char *get_tds_login_domainname(void);
 extern void TdsSetDbContext(void);
+extern void TdsResetLoginFlags(void);
 
 /* Functions in backend/tds/tdsprotocol.c */
 extern int	TdsSocketBackend(void);

diff --git a/contrib/babelfishpg_tsql/src/guc.c b/contrib/babelfishpg_tsql/src/guc.c
@@ -14,6 +14,7 @@
 
 #define PLTSQL_SESSION_ISOLATION_LEVEL "default_transaction_isolation"
 #define PLTSQL_TRANSACTION_ISOLATION_LEVEL "transaction_isolation"
+#define PLTSQL_MIGRATION_MODE "babelfishpg_tsql.migration_mode"
 #define PLTSQL_DEFAULT_LANGUAGE "us_english"
 
 static int	migration_mode = SINGLE_DB;
@@ -1657,7 +1658,9 @@ void
 pltsql_validate_set_config_function(char *name, char *value)
 {
 	if (strncmp(name, PLTSQL_SESSION_ISOLATION_LEVEL, strlen(PLTSQL_SESSION_ISOLATION_LEVEL)) == 0 ||
-		strncmp(name, PLTSQL_TRANSACTION_ISOLATION_LEVEL, strlen(PLTSQL_TRANSACTION_ISOLATION_LEVEL)) == 0)
+		strncmp(name, PLTSQL_TRANSACTION_ISOLATION_LEVEL, strlen(PLTSQL_TRANSACTION_ISOLATION_LEVEL)) == 0 ||
+		strncmp(name, PLTSQL_MIGRATION_MODE, strlen(PLTSQL_MIGRATION_MODE)) == 0 ||
+		strncmp(name, "role", strlen("role")) == 0)
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

diff --git a/contrib/babelfishpg_tsql/src/pl_exec-2.c b/contrib/babelfishpg_tsql/src/pl_exec-2.c
@@ -885,6 +885,17 @@ exec_stmt_exec(PLtsql_execstate *estate, PLtsql_stmt_exec *stmt)
 	else
 		estate->schema_name = NULL;
 
+	/*
+	 * We need to disable the explain gucs incase of sp_reset_connection
+	 * execution otherwise we will get explain output for it which is
+	 * not intended.
+	 */
+	if (strcmp(stmt->proc_name, "sp_reset_connection") == 0)
+	{
+		pltsql_explain_only = false;
+		pltsql_explain_analyze = false;
+	}
+
 	/* PG_TRY to ensure we clear the plan link, if needed, on failure */
 	PG_TRY();
 	{

diff --git a/contrib/babelfishpg_tsql/src/session.c b/contrib/babelfishpg_tsql/src/session.c
@@ -15,6 +15,7 @@
 #include "dbcmds.h"
 #include "multidb.h"
 #include "session.h"
+#include "pl_explain.h"
 #include "pltsql.h"
 #include "guc.h"
 #include "storage/shm_toc.h"
@@ -211,6 +212,8 @@ reset_session_properties(void)
 {
 	reset_cached_batch();
 	reset_cached_cursor();
+	pltsql_explain_only = false;
+	pltsql_explain_analyze = false;
 }
 
 void

diff --git a/test/JDBC/expected/Test-sp_reset_connection.out b/test/JDBC/expected/Test-sp_reset_connection.out
@@ -513,3 +513,212 @@ int
 
 drop table babel_cursor_t1;
 GO
+
+
+-- GUCs testing
+-- 1. Ansi defaults
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_nulls', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_warnings', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_null_dflt_on', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_padding', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.implicit_transactions', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.quoted_identifier', true);
+GO
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+off
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+
+SET ANSI_DEFAULTS ON
+GO
+
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_nulls', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_warnings', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_null_dflt_on', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_padding', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.implicit_transactions', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.quoted_identifier', true);
+GO
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+
+-- reset
+exec sys.sp_reset_connection
+GO
+
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_nulls', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_warnings', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_null_dflt_on', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.ansi_padding', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.implicit_transactions', true);
+SELECT CURRENT_SETTING('babelfishpg_tsql.quoted_identifier', true);
+GO
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+~~START~~
+text
+off
+~~END~~
+
+~~START~~
+text
+on
+~~END~~
+
+
+-- babelfish_showplan_all
+SET babelfish_showplan_all ON
+GO
+
+-- explain output
+SELECT 1;
+GO
+~~START~~
+text
+Query Text: SELECT 1
+Result  (cost=0.00..0.01 rows=1 width=4)
+~~END~~
+
+~~START~~
+text
+Babelfish T-SQL Batch Parsing Time: 0.085 ms
+~~END~~
+
+
+-- reset
+exec sys.sp_reset_connection
+GO
+
+-- 1 output
+SELECT 1;
+GO
+~~START~~
+int
+1
+~~END~~
+
+
+
+-- set_config testing.
+-- search_path has source < PGC_S_SESSION in TSQL but it gets reset during ResetAll Gucs.
+-- Whereas role does not get reset since it uses GUC_NO_RESET_ALL, so we should not allow
+-- set_config for this option.
+SELECT CURRENT_SETTING('search_path', true)
+SELECT CURRENT_SETTING('role', true)
+GO
+~~START~~
+text
+master_dbo, "$user", sys, pg_catalog
+~~END~~
+
+~~START~~
+text
+master_dbo
+~~END~~
+
+
+SELECT set_config('search_path', 'sys', false);
+GO
+~~START~~
+text
+sys
+~~END~~
+
+SELECT set_config('role', 'jdbc_user', false);
+GO
+~~START~~
+text
+~~ERROR (Code: 33557097)~~
+
+~~ERROR (Message: set_config not allowed for option role)~~
+
+
+-- reset
+exec sp_reset_connection
+GO
+
+SELECT CURRENT_SETTING('search_path', true)
+SELECT CURRENT_SETTING('role', true)
+GO
+~~START~~
+text
+master_dbo, "$user", sys, pg_catalog
+~~END~~
+
+~~START~~
+text
+master_dbo
+~~END~~
+