Skip to content

Commit

Permalink
[Rel14] Improve defensive code for crash in AtEOXact_Parallel (babelf…
Browse files Browse the repository at this point in the history
…ish-for-postgresql#2269)


Setting GUC during parallel operation will cause GUC inconsistency
because there is no way to sync the GUC value among parallel workers.

This commit only allows setting insert_identity during parallel
operation initialization.

Task: BABEL-4340/4419/4423
Signed-off-by: Xiaohui Fanhe <[email protected]>
  • Loading branch information
xhfanhe authored Jan 19, 2024
1 parent 18944b0 commit 61ca776
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 20 deletions.
26 changes: 15 additions & 11 deletions contrib/babelfishpg_tsql/src/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ static bool check_noexec (bool *newval, void **extra, GucSource source);
static bool check_showplan_all (bool *newval, void **extra, GucSource source);
static bool check_showplan_text (bool *newval, void **extra, GucSource source);
static bool check_showplan_xml (bool *newval, void **extra, GucSource source);
static bool check_enable_pg_hint(bool *newval, void **extra, GucSource source);
static void assign_transform_null_equals (bool newval, void *extra);
static void assign_ansi_defaults (bool newval, void *extra);
static void assign_quoted_identifier (bool newval, void *extra);
Expand Down Expand Up @@ -380,22 +381,15 @@ static bool check_tsql_version (char **newval, void **extra, GucSource source)
return true;
}

static void assign_enable_pg_hint (bool newval, void *extra)
static bool
check_enable_pg_hint(bool *newval, void **extra, GucSource source)
{
/*
* Parallel workers send data to the leader, not the client. They always
* send data using pg_hint_plan.enable_hint_plan.
*/
if (IsParallelWorker())
if (IsParallelWorker() && !InitializingParallelWorker)
{
/*
* 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
Expand All @@ -406,6 +400,16 @@ static void assign_enable_pg_hint (bool newval, void *extra)
errmsg("cannot change enable_hint_plan during a parallel operation")));
}

return true;
}


static void
assign_enable_pg_hint(bool newval, void *extra)
{
if (IsParallelWorker())
return;

if (newval)
{
/* Will throw an error if pg_hint_plan is not installed */
Expand Down Expand Up @@ -1098,7 +1102,7 @@ define_custom_variables(void)
false,
PGC_USERSET,
GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_DISALLOW_IN_AUTO_FILE,
NULL, assign_enable_pg_hint, NULL);
check_enable_pg_hint, assign_enable_pg_hint, NULL);

DefineCustomIntVariable("babelfishpg_tsql.insert_bulk_rows_per_batch",
gettext_noop("Sets the number of rows per batch to be processed for Insert Bulk"),
Expand Down
25 changes: 24 additions & 1 deletion contrib/babelfishpg_tsql/src/pl_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ extern bool pltsql_nocount;
extern List *babelfishpg_tsql_raw_parser(const char *str, RawParseMode mode);
extern bool install_backend_gram_hooks();

static bool check_identity_insert(char **newal, void **extra, GucSource source);
static void assign_identity_insert(const char *newval, void *extra);
static void assign_textsize(int newval, void *extra);
extern Datum init_collid_trans_tab(PG_FUNCTION_ARGS);
Expand Down Expand Up @@ -257,6 +258,28 @@ set_procid(Oid oid)
procid_var = oid;
}

static bool
check_identity_insert(char** newval, void **extra, GucSource source)
{
/*
* Workers synchronize the parameter at the beginning of each parallel
* operation. Avoid performing parameter assignment uring parallel operation.
*/
if (IsParallelWorker() && !InitializingParallelWorker)
{
/*
* 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 identity_insert during a parallel operation")));
}

return true;
}

static void
assign_identity_insert(const char *newval, void *extra)
{
Expand Down Expand Up @@ -3498,7 +3521,7 @@ _PG_init(void)
"",
PGC_USERSET,
GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_DISALLOW_IN_AUTO_FILE,
NULL,
check_identity_insert,
assign_identity_insert,
NULL);

Expand Down
8 changes: 0 additions & 8 deletions test/JDBC/parallel_query_jdbc_schedule
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@

ignore#!#table-variable-vu-verify

# JIRA - BABEL-4419
ignore#!#BABEL-1056
ignore#!#BABEL-GUC-PLAN
ignore#!#BABEL-3092
ignore#!#BABEL-COLUMN-CONSTRAINT
ignore#!#BABEL-1251-vu-verify
ignore#!#BABEL-1251

# These test should not get run in parallel query
ignore#!#BABEL-1363

Expand Down

0 comments on commit 61ca776

Please sign in to comment.