From abadf6b0848bab6fae37b6e8bfba377fb44019ec Mon Sep 17 00:00:00 2001 From: houzj Date: Tue, 16 Mar 2021 14:57:07 +0800 Subject: [PATCH] skip cci In RI check, we currently call CommandCounterIncrement every time we insert into a table with foreign key, which is not supported in a parallel worker. However, it's necessary to do CCI only if the referenced table is modified during an INSERT command. For now, all the cases that will modify the referenced table are treated as parallel unsafe. We can skip CCI to let the RI check (for now only RI_FKey_check_ins) to be executed in a parallel worker. For the parallel safety of RI_FKey_check_ins, it internally uses [select pk rel for key share] which is considered as parallel unsafe in planner. However, [select for key share] does not generate combo-cid or new command id. And it acquires share lock which does not conflict with itself and works as expected with group locking. In addition, as we assumed that the pk rel will not be modified, dead lock on pk rel will not happen. Based on above, IMO, execute [select for key share] in parallel worker is as safe as parallel insert. --- src/backend/executor/spi.c | 28 ++++++++++++---------- src/backend/utils/adt/ri_triggers.c | 48 ++++++++++++++++++++++++++----------- src/include/executor/spi.h | 3 ++- 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 00aa78e..f1dd775 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -69,7 +69,8 @@ static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, bool read_only, bool no_snapshots, bool fire_triggers, uint64 tcount, DestReceiver *caller_dest, - ResourceOwner plan_owner); + ResourceOwner plan_owner, + bool no_CCI); static ParamListInfo _SPI_convert_params(int nargs, Oid *argtypes, Datum *Values, const char *Nulls); @@ -525,7 +526,7 @@ SPI_execute(const char *src, bool read_only, long tcount) InvalidSnapshot, InvalidSnapshot, read_only, false, true, tcount, - NULL, NULL); + NULL, NULL, false); _SPI_end_call(true); return res; @@ -569,7 +570,7 @@ SPI_execute_extended(const char *src, InvalidSnapshot, InvalidSnapshot, options->read_only, options->no_snapshots, true, options->tcount, - options->dest, options->owner); + options->dest, options->owner, false); _SPI_end_call(true); return res; @@ -598,7 +599,7 @@ SPI_execute_plan(SPIPlanPtr plan, Datum *Values, const char *Nulls, InvalidSnapshot, InvalidSnapshot, read_only, false, true, tcount, - NULL, NULL); + NULL, NULL, false); _SPI_end_call(true); return res; @@ -629,7 +630,7 @@ SPI_execute_plan_extended(SPIPlanPtr plan, InvalidSnapshot, InvalidSnapshot, options->read_only, options->no_snapshots, true, options->tcount, - options->dest, options->owner); + options->dest, options->owner, false); _SPI_end_call(true); return res; @@ -653,7 +654,7 @@ SPI_execute_plan_with_paramlist(SPIPlanPtr plan, ParamListInfo params, InvalidSnapshot, InvalidSnapshot, read_only, false, true, tcount, - NULL, NULL); + NULL, NULL, false); _SPI_end_call(true); return res; @@ -676,7 +677,8 @@ int SPI_execute_snapshot(SPIPlanPtr plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, bool fire_triggers, long tcount) + bool read_only, bool fire_triggers, long tcount, + bool no_CCI) { int res; @@ -696,7 +698,7 @@ SPI_execute_snapshot(SPIPlanPtr plan, snapshot, crosscheck_snapshot, read_only, false, fire_triggers, tcount, - NULL, NULL); + NULL, NULL, no_CCI); _SPI_end_call(true); return res; @@ -746,7 +748,7 @@ SPI_execute_with_args(const char *src, InvalidSnapshot, InvalidSnapshot, read_only, false, true, tcount, - NULL, NULL); + NULL, NULL, false); _SPI_end_call(true); return res; @@ -2271,13 +2273,15 @@ _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan) * caller_dest: DestReceiver to receive output, or NULL for normal SPI output * plan_owner: ResourceOwner that will be used to hold refcount on plan; * if NULL, CurrentResourceOwner is used (ignored for non-saved plan) + * no_CCI: true to skip CommandCounterIncrement even if read_only is false */ static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, Snapshot snapshot, Snapshot crosscheck_snapshot, bool read_only, bool no_snapshots, bool fire_triggers, uint64 tcount, - DestReceiver *caller_dest, ResourceOwner plan_owner) + DestReceiver *caller_dest, ResourceOwner plan_owner, + bool no_CCI) { int my_res = 0; uint64 my_processed = 0; @@ -2464,7 +2468,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, * If not read-only mode, advance the command counter before each * command and update the snapshot. */ - if (!read_only && !no_snapshots) + if (!no_CCI && !read_only && !no_snapshots) { CommandCounterIncrement(); UpdateActiveSnapshotCommandId(); @@ -2608,7 +2612,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, * command. This ensures that its effects are visible, in case it was * DDL that would affect the next CachedPlanSource. */ - if (!read_only) + if (!no_CCI && !read_only) CommandCounterIncrement(); } diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 09a2ad2..959ff1f 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -217,7 +217,7 @@ static bool ri_PerformCheck(const RI_ConstraintInfo *riinfo, RI_QueryKey *qkey, SPIPlanPtr qplan, Relation fk_rel, Relation pk_rel, TupleTableSlot *oldslot, TupleTableSlot *newslot, - bool detectNewRows, int expect_OK); + bool detectNewRows, int expect_OK, bool modifypk); static void ri_ExtractValues(Relation rel, TupleTableSlot *slot, const RI_ConstraintInfo *riinfo, bool rel_is_pk, Datum *vals, char *nulls); @@ -233,7 +233,7 @@ static void ri_ReportViolation(const RI_ConstraintInfo *riinfo, * Check foreign key existence (combined for INSERT and UPDATE). */ static Datum -RI_FKey_check(TriggerData *trigdata) +RI_FKey_check(TriggerData *trigdata, bool modifypk) { const RI_ConstraintInfo *riinfo; Relation fk_rel; @@ -397,7 +397,8 @@ RI_FKey_check(TriggerData *trigdata) fk_rel, pk_rel, NULL, newslot, false, - SPI_OK_SELECT); + SPI_OK_SELECT, + modifypk); if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); @@ -416,11 +417,25 @@ RI_FKey_check(TriggerData *trigdata) Datum RI_FKey_check_ins(PG_FUNCTION_ARGS) { + bool modifypk = true; + /* Check that this is a valid trigger call on the right time and event. */ ri_CheckTrigger(fcinfo, "RI_FKey_check_ins", RI_TRIGTYPE_INSERT); + /* + * We do not allow parallel insert if the pk relation could be modified + * during INSERT command, because it needs to call CommandCounterIncrement + * to let the modifications on pk relation visible for the RI check which + * is not support in parallel worker. + * + * (Note: Once we support CCI in parallel worker, this dependency is no + * longer needed). + */ + if (IsInParallelMode()) + modifypk = false; + /* Share code with UPDATE case. */ - return RI_FKey_check((TriggerData *) fcinfo->context); + return RI_FKey_check((TriggerData *) fcinfo->context, modifypk); } @@ -436,7 +451,7 @@ RI_FKey_check_upd(PG_FUNCTION_ARGS) ri_CheckTrigger(fcinfo, "RI_FKey_check_upd", RI_TRIGTYPE_UPDATE); /* Share code with INSERT case. */ - return RI_FKey_check((TriggerData *) fcinfo->context); + return RI_FKey_check((TriggerData *) fcinfo->context, true); } @@ -524,7 +539,8 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel, fk_rel, pk_rel, oldslot, NULL, true, /* treat like update */ - SPI_OK_SELECT); + SPI_OK_SELECT, + true); if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); @@ -716,7 +732,8 @@ ri_restrict(TriggerData *trigdata, bool is_no_action) fk_rel, pk_rel, oldslot, NULL, true, /* must detect new rows */ - SPI_OK_SELECT); + SPI_OK_SELECT, + true); if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); @@ -822,7 +839,8 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS) fk_rel, pk_rel, oldslot, NULL, true, /* must detect new rows */ - SPI_OK_DELETE); + SPI_OK_DELETE, + true); if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); @@ -943,7 +961,8 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS) fk_rel, pk_rel, oldslot, newslot, true, /* must detect new rows */ - SPI_OK_UPDATE); + SPI_OK_UPDATE, + true); if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); @@ -1122,7 +1141,8 @@ ri_set(TriggerData *trigdata, bool is_set_null) fk_rel, pk_rel, oldslot, NULL, true, /* must detect new rows */ - SPI_OK_UPDATE); + SPI_OK_UPDATE, + true); if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); @@ -1496,7 +1516,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) NULL, NULL, GetLatestSnapshot(), InvalidSnapshot, - true, false, 1); + true, false, 1, false); /* Check result */ if (spi_result != SPI_OK_SELECT) @@ -1736,7 +1756,7 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) NULL, NULL, GetLatestSnapshot(), InvalidSnapshot, - true, false, 1); + true, false, 1, false); /* Check result */ if (spi_result != SPI_OK_SELECT) @@ -2238,7 +2258,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, RI_QueryKey *qkey, SPIPlanPtr qplan, Relation fk_rel, Relation pk_rel, TupleTableSlot *oldslot, TupleTableSlot *newslot, - bool detectNewRows, int expect_OK) + bool detectNewRows, int expect_OK, bool modifypk) { Relation query_rel, source_rel; @@ -2336,7 +2356,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, spi_result = SPI_execute_snapshot(qplan, vals, nulls, test_snapshot, crosscheck_snapshot, - false, false, limit); + false, false, limit, !modifypk); /* Restore UID and security context */ SetUserIdAndSecContext(save_userid, save_sec_context); diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index 6455d10..7150199 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -129,7 +129,8 @@ extern int SPI_execute_snapshot(SPIPlanPtr plan, Datum *Values, const char *Nulls, Snapshot snapshot, Snapshot crosscheck_snapshot, - bool read_only, bool fire_triggers, long tcount); + bool read_only, bool fire_triggers, long tcount, + bool no_CCI); extern int SPI_execute_with_args(const char *src, int nargs, Oid *argtypes, Datum *Values, const char *Nulls, -- 2.7.2.windows.1