From d1be68097b39ae9f9b5469cb963d012b5ba51e4c Mon Sep 17 00:00:00 2001 From: Karthik Ramanathan Date: Mon, 14 Oct 2024 15:31:13 -0700 Subject: [PATCH] [BACKPORT 2024.2][#20908] YSQL: Gate inplace index updates behind feature enablement GUC Summary: D36588 introduced an optimization that enabled updates to non-key columns of secondary indexes to be performed in-place (ie. not require a DELETE + INSERT). This optimization was ON by default and was not gated behind a feature flag. This revision introduces a Postgres GUC `yb_enable_inplace_index_update` to enable/disable the feature. By default the GUC is set to true, keeping the optimization turned ON. Jira: DB-9891 Original commit: 73eb9577c5bc26dce19a0bc36a85792215d0fbad / D38763 Test Plan: Run the following tests: ``` ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressUpdateOptimized#schedule' ``` Reviewers: smishra, amartsinchyk, tnayak Reviewed By: smishra Subscribers: yql Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D39029 --- src/postgres/src/backend/executor/execIndexing.c | 13 +++++++------ src/postgres/src/backend/executor/nodeModifyTable.c | 5 ++++- src/postgres/src/backend/utils/misc/guc.c | 13 +++++++++++++ src/postgres/src/backend/utils/misc/pg_yb_utils.c | 1 + src/postgres/src/include/executor/executor.h | 3 ++- src/postgres/src/include/nodes/execnodes.h | 7 +++++++ src/postgres/src/include/pg_yb_utils.h | 6 ++++++ 7 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/postgres/src/backend/executor/execIndexing.c b/src/postgres/src/backend/executor/execIndexing.c index 68e837c49892..02fcb926c16b 100644 --- a/src/postgres/src/backend/executor/execIndexing.c +++ b/src/postgres/src/backend/executor/execIndexing.c @@ -765,7 +765,8 @@ YbExecUpdateIndexTuples(TupleTableSlot *slot, HeapTuple tuple, EState *estate, Bitmapset *updatedCols, - bool is_pk_updated) + bool is_pk_updated, + bool is_inplace_update_enabled) { ResultRelInfo *resultRelInfo; int i; @@ -863,7 +864,7 @@ YbExecUpdateIndexTuples(TupleTableSlot *slot, econtext->ecxt_scantuple = slot; insertApplicable = ExecQual(predicate, econtext); - + if (deleteApplicable != insertApplicable) { /* @@ -878,8 +879,7 @@ YbExecUpdateIndexTuples(TupleTableSlot *slot, continue; } - - + if (!deleteApplicable) { /* Neither deletes nor updates applicable. Nothing to be done for this index. */ @@ -915,7 +915,8 @@ YbExecUpdateIndexTuples(TupleTableSlot *slot, } } - if (!indexRelation->rd_amroutine->ybamcanupdatetupleinplace) + if (!(is_inplace_update_enabled && + indexRelation->rd_amroutine->ybamcanupdatetupleinplace)) { deleteIndexes = lappend_int(deleteIndexes, i); insertIndexes = lappend_int(insertIndexes, i); @@ -968,7 +969,7 @@ YbExecUpdateIndexTuples(TupleTableSlot *slot, * * To achieve this, we compute the list of all indexes whose key columns * are updated. These need the DELETE + INSERT. For all indexes, first - * issue the deletes, followed by the inserts. + * issue the deletes, followed by the inserts. */ int j = 0; diff --git a/src/postgres/src/backend/executor/nodeModifyTable.c b/src/postgres/src/backend/executor/nodeModifyTable.c index 2c03c95fddbd..e674af9838ce 100644 --- a/src/postgres/src/backend/executor/nodeModifyTable.c +++ b/src/postgres/src/backend/executor/nodeModifyTable.c @@ -1642,7 +1642,8 @@ ExecUpdate(ModifyTableState *mtstate, { recheckIndexes = YbExecUpdateIndexTuples( slot, YBCGetYBTupleIdFromSlot(planSlot), oldtuple, tuple, - estate, cols_marked_for_update, is_pk_updated); + estate, cols_marked_for_update, is_pk_updated, + mtstate->yb_is_inplace_index_update_enabled); } bms_free(cols_marked_for_update); @@ -2996,6 +2997,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans); mtstate->mt_nplans = nplans; + mtstate->yb_is_inplace_index_update_enabled = yb_enable_inplace_index_update; + /* set up epqstate with dummy subplan data for the moment */ EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL, node->epqParam); mtstate->fireBSTriggers = true; diff --git a/src/postgres/src/backend/utils/misc/guc.c b/src/postgres/src/backend/utils/misc/guc.c index c652bbfe8bb4..341ce0663ad1 100644 --- a/src/postgres/src/backend/utils/misc/guc.c +++ b/src/postgres/src/backend/utils/misc/guc.c @@ -2665,6 +2665,19 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"yb_enable_inplace_index_update", PGC_USERSET, QUERY_TUNING_OTHER, + gettext_noop("Enables the in-place update of non-key columns of secondary indexes " + "when key columns of the index are not updated. This is useful when " + "updating the included columns in a covering index among others."), + NULL, + GUC_NOT_IN_SAMPLE + }, + &yb_enable_inplace_index_update, + true, + NULL, NULL, NULL + }, + { {"yb_enable_fkey_catcache", PGC_USERSET, DEVELOPER_OPTIONS, gettext_noop("Enable preloading of foreign key information into the relation cache."), diff --git a/src/postgres/src/backend/utils/misc/pg_yb_utils.c b/src/postgres/src/backend/utils/misc/pg_yb_utils.c index 5cf10269bc02..f4d1c614619d 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -1390,6 +1390,7 @@ int yb_toast_catcache_threshold = -1; int yb_parallel_range_size = 1024 * 1024; bool yb_enable_fkey_catcache = true; int yb_insert_on_conflict_read_batch_size = 1024; +bool yb_enable_inplace_index_update = true; YBUpdateOptimizationOptions yb_update_optimization_options = { .has_infra = true, diff --git a/src/postgres/src/include/executor/executor.h b/src/postgres/src/include/executor/executor.h index b4aa98504cc4..fcd44fa065cc 100644 --- a/src/postgres/src/include/executor/executor.h +++ b/src/postgres/src/include/executor/executor.h @@ -601,7 +601,8 @@ extern List *YbExecUpdateIndexTuples(TupleTableSlot *slot, HeapTuple tuple, EState *estate, Bitmapset *updatedCols, - bool is_pk_updated); + bool is_pk_updated, + bool is_inplace_update_enabled); extern bool ExecCheckIndexConstraints(TupleTableSlot *slot, EState *estate, ItemPointer conflictTid, List *arbiterIndexes, diff --git a/src/postgres/src/include/nodes/execnodes.h b/src/postgres/src/include/nodes/execnodes.h index be12ac2090c4..cfd70e5589a0 100644 --- a/src/postgres/src/include/nodes/execnodes.h +++ b/src/postgres/src/include/nodes/execnodes.h @@ -1200,6 +1200,13 @@ typedef struct ModifyTableState * constraint checks etc. This field is set to false for single row txns. */ bool yb_is_update_optimization_enabled; + + /* + * If enabled, execution seeks to perform inplace update of non-key columns + * of secondary indexes. This field is not applicable to single row txns + * because they do not involve updates to secondary indexes. + */ + bool yb_is_inplace_index_update_enabled; } ModifyTableState; /* ---------------- diff --git a/src/postgres/src/include/pg_yb_utils.h b/src/postgres/src/include/pg_yb_utils.h index 216dccd5b7ab..d33a5fe0ff2c 100644 --- a/src/postgres/src/include/pg_yb_utils.h +++ b/src/postgres/src/include/pg_yb_utils.h @@ -666,6 +666,12 @@ YbDdlRollbackEnabled () { extern bool yb_use_hash_splitting_by_default; +/* + * If set to true, non-key columns of secondary indexes are updated in-place + * when no key columns are modified. + */ +extern bool yb_enable_inplace_index_update; + typedef struct YBUpdateOptimizationOptions { bool has_infra;