From 73eb9577c5bc26dce19a0bc36a85792215d0fbad Mon Sep 17 00:00:00 2001 From: Karthik Ramanathan Date: Mon, 14 Oct 2024 15:02:45 -0700 Subject: [PATCH] [#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 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/D38763 --- src/postgres/src/backend/executor/execIndexing.c | 13 +++++++------ src/postgres/src/backend/executor/nodeModifyTable.c | 4 +++- 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, 39 insertions(+), 8 deletions(-) diff --git a/src/postgres/src/backend/executor/execIndexing.c b/src/postgres/src/backend/executor/execIndexing.c index cccaa6cd8ff6..666a107e8d85 100644 --- a/src/postgres/src/backend/executor/execIndexing.c +++ b/src/postgres/src/backend/executor/execIndexing.c @@ -785,7 +785,8 @@ YbExecUpdateIndexTuples(ResultRelInfo *resultRelInfo, ItemPointer tupleid, EState *estate, Bitmapset *updatedCols, - bool is_pk_updated) + bool is_pk_updated, + bool is_inplace_update_enabled) { int i; int numIndices; @@ -882,7 +883,7 @@ YbExecUpdateIndexTuples(ResultRelInfo *resultRelInfo, econtext->ecxt_scantuple = slot; insertApplicable = ExecQual(predicate, econtext); - + if (deleteApplicable != insertApplicable) { /* @@ -897,8 +898,7 @@ YbExecUpdateIndexTuples(ResultRelInfo *resultRelInfo, continue; } - - + if (!deleteApplicable) { /* Neither deletes nor updates applicable. Nothing to be done for this index. */ @@ -934,7 +934,8 @@ YbExecUpdateIndexTuples(ResultRelInfo *resultRelInfo, } } - if (!indexRelation->rd_indam->ybamcanupdatetupleinplace) + if (!(is_inplace_update_enabled && + indexRelation->rd_indam->ybamcanupdatetupleinplace)) { deleteIndexes = lappend_int(deleteIndexes, i); insertIndexes = lappend_int(insertIndexes, i); @@ -987,7 +988,7 @@ YbExecUpdateIndexTuples(ResultRelInfo *resultRelInfo, * * 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 72645c9a86bb..3a7a34536156 100644 --- a/src/postgres/src/backend/executor/nodeModifyTable.c +++ b/src/postgres/src/backend/executor/nodeModifyTable.c @@ -2456,7 +2456,7 @@ ExecUpdateEpilogue(ModifyTableContext *context, UpdateContext *updateCxt, recheckIndexes = YbExecUpdateIndexTuples( resultRelInfo, slot, YBCGetYBTupleIdFromSlot(context->planSlot), oldtuple, tupleid, context->estate, yb_cols_marked_for_update, - yb_is_pk_updated); + yb_is_pk_updated, mtstate->yb_is_inplace_index_update_enabled); } } else @@ -4564,6 +4564,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) (node->yb_update_affected_entities != NULL && operation == CMD_UPDATE && YbIsUpdateOptimizationEnabled()); + 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 b6c245e596d9..709a6092114a 100644 --- a/src/postgres/src/backend/utils/misc/guc.c +++ b/src/postgres/src/backend/utils/misc/guc.c @@ -2979,6 +2979,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 b3d4d19d9e41..c6f1ecdff717 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -1640,6 +1640,7 @@ int yb_parallel_range_size = 1024 * 1024; int yb_insert_on_conflict_read_batch_size = 1024; bool yb_enable_fkey_catcache = true; bool yb_enable_nop_alter_role_optimization = true; +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 f52ff4fe72c9..b18d02d80fa8 100644 --- a/src/postgres/src/include/executor/executor.h +++ b/src/postgres/src/include/executor/executor.h @@ -663,7 +663,8 @@ extern List *YbExecUpdateIndexTuples(ResultRelInfo *resultRelInfo, ItemPointer tupleid, EState *estate, Bitmapset *updatedCols, - bool is_pk_updated); + bool is_pk_updated, + bool is_inplace_update_enabled); /* * prototypes from functions in execReplication.c diff --git a/src/postgres/src/include/nodes/execnodes.h b/src/postgres/src/include/nodes/execnodes.h index 52614979b775..8892c28a5088 100644 --- a/src/postgres/src/include/nodes/execnodes.h +++ b/src/postgres/src/include/nodes/execnodes.h @@ -1404,6 +1404,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 66b677e1abb2..4b5df036effd 100644 --- a/src/postgres/src/include/pg_yb_utils.h +++ b/src/postgres/src/include/pg_yb_utils.h @@ -708,6 +708,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;