fix crash in parallel query with enable_pg_hint 'on' 'server' (babelf…

…ish-for-postgresql#1690)

When enable_pg_hint is set to 'on', any query which leads to creation of parallel worker crashes. This is happening because in initialization of parallel worker while restoring the library state of backend to parallel worker, GUC state variable pg_hint_plan.enable_hint is getting set from the assign function of the custom defined variable babelfishpg_tsql.enable_pg_hint, which later in RestoreGUCState function causes an assert failure as it expects all the guc variables of parallel worker to be NULL to begin with, hence crash is happening.
This issue is resolved by adding a check in the assign_enable_pg_hint which prevent the parallel worker from setting the GUC variable pg_hint_plan.enable_hint.

Task: BABEL-4294
Sign-off-by: Rohit Bhagat <[email protected]>

contrib/babelfishpg_tsql/src/guc.c

@@ -10,6 +10,7 @@
 #include "pltsql.h"
 #include "pl_explain.h"
 #include "miscadmin.h"
+#include "access/parallel.h"
 
 #define PLTSQL_SESSION_ISOLATION_LEVEL "default_transaction_isolation"
 #define PLTSQL_TRANSACTION_ISOLATION_LEVEL "transaction_isolation"
@@ -392,6 +393,30 @@ check_tsql_version(char **newval, void **extra, GucSource source)
 static void
 assign_enable_pg_hint(bool newval, void *extra)
 {
+	/*
+     * Parallel workers send data to the leader, not the client. They always
+     * send data using pg_hint_plan.enable_hint_plan.
+     */
+    if (IsParallelWorker())
+    {
+        /*
+         * During parallel worker startup, we want to accept the leader's
+         * hint_plan setting so that anyone who looks at the value in
+         * the worker sees the same value that they would see in the leader.
+         */
+        if (InitializingParallelWorker)
+            return;
+
+        /*
+         * A change other than during startup, for example due to a SET clause
+         * attached to a function definition, should be rejected, as there is
+         * nothing we can do inside the worker to make it take effect.
+         */
+        ereport(ERROR,
+                (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
+                 errmsg("cannot change enable_hint_plan during a parallel operation")));
+    }
+
 	if (newval)
 	{
 		/* Will throw an error if pg_hint_plan is not installed */

test/JDBC/expected/BABEL-4294-vu-cleanup.out

@@ -0,0 +1,9 @@
+
+-- Test to check if initialisation of Parallel Worker crash when babelfishpg_tsql.enable_pg_hint is set
+drop table babel_4294_t1;
+drop table babel_4294_t2;
+drop table babel_4294_t3;
+go
+
+drop table babel_4294_t4;
+go

test/JDBC/expected/BABEL-4294-vu-prepare.out

@@ -0,0 +1,20 @@
+
+-- Test to check if initialisation of Parallel Worker crash when babelfishpg_tsql.enable_pg_hint is set
+create table babel_4294_t1(id INT, val int);
+create table babel_4294_t2(babel_4294_t1_id INT, val int);
+create table babel_4294_t3(babel_4294_t1_id INT, val int);
+go
+
+insert into babel_4294_t1 values (1, 10), (2, 20), (3, 30);
+insert into babel_4294_t2 values (1, 11), (2, 12), (3, 13);
+insert into babel_4294_t3 values (1, 99), (2, 77), (3, 55);
+go
+~~ROW COUNT: 3~~
+
+~~ROW COUNT: 3~~
+
+~~ROW COUNT: 3~~
+
+
+create table babel_4294_t4(id INT, val int);
+go

test/JDBC/expected/BABEL-4294-vu-verify.out

@@ -0,0 +1,56 @@
+
+-- Test to check if initialisation of Parallel Worker crash when babelfishpg_tsql.enable_pg_hint is set
+/*
+ * Set the enable_pg_hint, try to create parallel worker
+ */
+exec sp_babelfish_configure 'enable_pg_hint', 'on', 'server'
+go
+
+select COUNT( babel_4294_t3.val), babel_4294_t2.val from babel_4294_t1
+inner join babel_4294_t2 on babel_4294_t1.id = babel_4294_t2.babel_4294_t1_id
+inner join babel_4294_t3 on babel_4294_t1.id = babel_4294_t3.babel_4294_t1_id
+GROUP BY babel_4294_t2.val
+UNION ALL
+select COUNT( babel_4294_t3.val), babel_4294_t2.val from babel_4294_t1
+inner join babel_4294_t2 on babel_4294_t1.id = babel_4294_t2.babel_4294_t1_id
+inner join babel_4294_t3 on babel_4294_t1.id = babel_4294_t3.babel_4294_t1_id
+GROUP BY babel_4294_t2.val
+go
+~~START~~
+int#!#int
+1#!#11
+1#!#13
+1#!#12
+1#!#11
+1#!#13
+1#!#12
+~~END~~
+
+
+-- Used force parallel mode to create a parallel worker
+select set_config('force_parallel_mode', '1', false)
+go
+~~START~~
+text
+on
+~~END~~
+
+
+-- to check if parallel worker generated for following query, will crash or not
+select * from babel_4294_t4
+go
+~~START~~
+int#!#int
+~~END~~
+
+
+select set_config('force_parallel_mode', '0', false)
+go
+~~START~~
+text
+off
+~~END~~
+
+
+exec sp_babelfish_configure 'enable_pg_hint', 'off', 'server'
+go

test/JDBC/input/pg_hint_plan/BABEL-4294-vu-cleanup.sql

@@ -0,0 +1,9 @@
+-- Test to check if initialisation of Parallel Worker crash when babelfishpg_tsql.enable_pg_hint is set
+
+drop table babel_4294_t1;
+drop table babel_4294_t2;
+drop table babel_4294_t3;
+go
+
+drop table babel_4294_t4;
+go

test/JDBC/input/pg_hint_plan/BABEL-4294-vu-prepare.sql

@@ -0,0 +1,14 @@
+-- Test to check if initialisation of Parallel Worker crash when babelfishpg_tsql.enable_pg_hint is set
+
+create table babel_4294_t1(id INT, val int);
+create table babel_4294_t2(babel_4294_t1_id INT, val int);
+create table babel_4294_t3(babel_4294_t1_id INT, val int);
+go
+
+insert into babel_4294_t1 values (1, 10), (2, 20), (3, 30);
+insert into babel_4294_t2 values (1, 11), (2, 12), (3, 13);
+insert into babel_4294_t3 values (1, 99), (2, 77), (3, 55);
+go
+
+create table babel_4294_t4(id INT, val int);
+go

test/JDBC/input/pg_hint_plan/BABEL-4294-vu-verify.sql

@@ -0,0 +1,32 @@
+-- Test to check if initialisation of Parallel Worker crash when babelfishpg_tsql.enable_pg_hint is set
+/*
+ * Set the enable_pg_hint, try to create parallel worker
+ */
+
+exec sp_babelfish_configure 'enable_pg_hint', 'on', 'server'
+go
+
+select COUNT( babel_4294_t3.val), babel_4294_t2.val from babel_4294_t1
+inner join babel_4294_t2 on babel_4294_t1.id = babel_4294_t2.babel_4294_t1_id
+inner join babel_4294_t3 on babel_4294_t1.id = babel_4294_t3.babel_4294_t1_id
+GROUP BY babel_4294_t2.val
+UNION ALL
+select COUNT( babel_4294_t3.val), babel_4294_t2.val from babel_4294_t1
+inner join babel_4294_t2 on babel_4294_t1.id = babel_4294_t2.babel_4294_t1_id
+inner join babel_4294_t3 on babel_4294_t1.id = babel_4294_t3.babel_4294_t1_id
+GROUP BY babel_4294_t2.val
+go
+
+-- Used force parallel mode to create a parallel worker
+select set_config('force_parallel_mode', '1', false)
+go
+
+-- to check if parallel worker generated for following query, will crash or not
+select * from babel_4294_t4
+go
+
+select set_config('force_parallel_mode', '0', false)
+go
+
+exec sp_babelfish_configure 'enable_pg_hint', 'off', 'server'
+go