[#18822] YSQL: Framework to skip redundant sec index updates and fkey checks when relevant columns not modified

Summary:
**Background**
Prior to this revision, an UPDATE statement specifying a list of target columns X in its SET clause, **always** performed the necessary work to update each of the target columns in the storage layer, irrespective of whether the values of the columns actually changed. The necessary work could include requiring locks, updating indexes, checking of constraints, firing of triggers etc.

**The Optimization**
This revision introduces an optimization that validates that the values of a column are indeed being modified, before sending (flushing) the updated value of the column to the storage layer.
In particular, the set of columns whose values that are compared are those that can cause extra round trips to the storage layer in the form of:
 - Primary Key Updates
 - Secondary Index Updates
 - Foreign Key Constraints
 - Uniqueness Constraints

The matrix of columns that are marked for update and the objects (indexes, constraints) they impact are computed at planning time.
This is particularly useful when used in conjunction with prepared statements and ORMs, which tend to specify all columns (both modified and non-modified) as part of the target list.
The decision of whether a column is indeed modified is done on a per-tuple basis at execution time.

**Example**
As a concrete example, consider a table with the following schema and data:
```
yugabyte=# CREATE TABLE foo (h INT PRIMARY KEY, v1 INT, v2 INT, v3 INT);
yugabyte=# CREATE INDEX foo_v1_idx ON foo (v1);
yugabyte=# CREATE INDEX foo_v2_idx ON foo (v2);
yugabyte=# INSERT INTO foo (SELECT i, i, i % 10, i % 100 FROM generate_series(1, 10000) AS i);
```

Performing an UPDATE on the first 1000 rows (without the optimization) yields:
```
yugabyte=# SET yb_explain_hide_non_deterministic_fields TO true;
yugabyte=# EXPLAIN (ANALYZE, DIST) UPDATE foo SET h = v1, v1 = v1, v3 = v3 + 1 WHERE v1 <= 1000;
                                        QUERY PLAN
------------------------------------------------------------------------------------------
 Update on foo  (cost=0.00..105.00 rows=1000 width=88) (actual rows=0 loops=1)
   ->  Seq Scan on foo  (cost=0.00..105.00 rows=1000 width=88) (actual rows=1000 loops=1)
         Remote Filter: (v1 <= 1000)
         Storage Table Read Requests: 1
         Storage Table Rows Scanned: 10000
         Storage Table Write Requests: 2000
         Storage Index Write Requests: 4000
         Storage Flush Requests: 2000
 Storage Read Requests: 1
 Storage Rows Scanned: 10000
 Storage Write Requests: 6000
 Storage Flush Requests: 2001
(12 rows)
```

The values of `h` and `v1` are not modified by the query, yet result in multiple write requests to both the main table as well as the secondary indices.
Since updates to key columns (of a table or an index) is executed as a sequence of a DELETE followed by an INSERT, this query requires a large amount of flushes.
This makes the query very expensive in terms of the amount of work to be done.
With the proposed optimization the query is executed as follows:
```
yugabyte=# EXPLAIN (ANALYZE, DIST) UPDATE foo SET h = v1, v1 = v1, v3 = v3 + 1 WHERE v1 <= 1000;
                                        QUERY PLAN
------------------------------------------------------------------------------------------
 Update on foo  (cost=0.00..105.00 rows=1000 width=88) (actual rows=0 loops=1)
   ->  Seq Scan on foo  (cost=0.00..105.00 rows=1000 width=88) (actual rows=1000 loops=1)
         Remote Filter: (v1 <= 1000)
         Storage Table Read Requests: 1
         Storage Rows Scanned: 10000
         Storage Table Write Requests: 1000
 Storage Read Requests: 1
 Storage Rows Scanned: 10000
 Storage Write Requests: 1000
 Storage Flush Requests: 1
(10 rows)
```

**Flags and Feature Status**
This revision introduces the following GUCs to control the behavior of this optimization:
`yb_update_num_cols_to_compare` - The maximum number of columns to be compared. (default: 0)
`yb_update_max_cols_size_to_compare` - The maximum size of an individual column that can be compared. (default: 10240)

This feature is currently turned off as a result of setting `yb_update_num_cols_to_compare` to 0.

**Debuggability**
Turn on postgres debug2 logs via the following command:
```
./bin/yb-ctl restart --ysql_pg_conf_csv='log_min_messages=debug2'
```

This produces the following debug information:
```
-- At planning time
2024-07-31 10:59:07.124 PDT [76120] DEBUG: Update matrix: rows represent OID of entities, columns represent attnum of cols
2024-07-31 10:59:07.124 PDT [76120] DEBUG:  -		10
2024-07-31 10:59:07.124 PDT [76120] DEBUG:  17415	Y

-- At execution time, on a per-tuple basis
2024-07-31 10:59:07.143 PDT [76120] DEBUG:  Index/constraint with oid 17415 requires an update
2024-07-31 10:59:07.143 PDT [76120] DEBUG:  Relation: 17412	Columns that are inspected and modified: 1 (10)
2024-07-31 10:59:07.143 PDT [76120] DEBUG:  No cols in category: Columns that are inspected and unmodified
2024-07-31 10:59:07.143 PDT [76120] DEBUG:  Relation: 17412	Columns that are marked for update: 1 (10) 2 (11)
```

**Future Work**
 1. Introduce auto-flag infrastructure to safely use row-locking. This is in the context of upgrade safety while the cluster is being upgraded.
 2. As a part of the flag infrastructure, ensure that flags/GUC values are immutable during the lifetime of a query.
 3. #22994: PGSQL_UPDATEs with no column references should acquire row locks.
 4. #23348: Add support for partitioned tables with out of order columns.
 5. Support for serializing optimization metadata in plans.
 6. Enhance randgen grammar to support ModifyTable (INSERT/UPDATE/DELETE ) queries
 7. #23350: PG 15 support.

jenkins: urgent
Jira: DB-7701

Test Plan:
Run the associated pg_regress test as follows:
```
# New tests
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressUpdateOptimized#schedule'

# Existing tests
./yb_build.sh --java-test 'org.yb.pgsql.TestPgUpdatePrimaryKey'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgUniqueConstraint'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTrigger#testPgRegressTrigger'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressDml#testPgRegressDml'
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressPushdown#testPgRegressPushdown'
```

Tested scenarios include (but not limited to):
1. Single row and distributed transactions with and without the feature flag turned on.
2. Relations with a primary key and no secondary indexes or triggers (UPDATEs can take the single row path)
3. Relations with combinations of primary key and secondary indexes.
4. Relations with unconditional before-row triggers.
5. UPDATEs in Colocated databases.
6. UPDATEs covering multiple tuples.
7. Hierarchy of relations with foreign keys
8. Relations with self referential foreign keys
9. Relations with overlapping indexes.
10. Relations having columns with uniqueness constraints.
11. Relations having covering indexes.
12. Relations having partial indexes.
13. Relations having index expressions / predicates.
14. Relations with conditional column triggers.
15. Relations having indexes/constraints out of order (ie. order of columns in relation is different from that of entity)
16. Relations having combination of hash and range indexes.
17. UPDATEs with correlated subqueries.
18. INSERT ON CONFLICT DO UPDATE.
19. UPDATE RETURNING.
20. UPDATEs on temp tables.

Reviewers: mihnea, jason, amartsinchyk

Reviewed By: amartsinchyk

Subscribers: pjain, jason, smishra, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D34040

Author: karthik-ramanathan-3006

java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressUpdateOptimized.java

@@ -0,0 +1,39 @@
+// Copyright (c) YugabyteDB, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+// in compliance with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software distributed under the License
+// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+// or implied.  See the License for the specific language governing permissions and limitations
+// under the License.
+//
+package org.yb.pgsql;
+
+import java.util.Map;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.yb.YBTestRunner;
+
+@RunWith(value=YBTestRunner.class)
+public class TestPgRegressUpdateOptimized extends BasePgRegressTest {
+  @Override
+  public int getTestMethodTimeoutSec() {
+    return 1800;
+  }
+
+  @Override
+  protected Map<String, String> getTServerFlags() {
+    Map<String, String> flagMap = super.getTServerFlags();
+    flagMap.put("ysql_skip_row_lock_for_update", "false");
+    return flagMap;
+  }
+
+  @Test
+  public void schedule() throws Exception {
+    runPgRegressTest("yb_update_optimized_schedule");
+  }
+}

src/postgres/src/backend/commands/trigger.c

@@ -6054,7 +6054,8 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
 				case RI_TRIGGER_PK:
 					/* Update on trigger's PK table */
 					if (!RI_FKey_pk_upd_check_required(trigger, rel,
-													   oldtup, newtup))
+													   oldtup, newtup,
+													   &estate->yb_skip_entities))
 					{
 						/* skip queuing this event */
 						continue;
@@ -6064,7 +6065,8 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
 				case RI_TRIGGER_FK:
 					/* Update on trigger's FK table */
 					if (!RI_FKey_fk_upd_check_required(trigger, rel,
-													   oldtup, newtup))
+													   oldtup, newtup,
+													   &estate->yb_skip_entities))
 					{
 						/* skip queuing this event */
 						continue;

src/postgres/src/backend/executor/Makefile

@@ -31,6 +31,7 @@ OBJS = execAmi.o execCurrent.o execExpr.o execExprInterp.o \
        nodeGroup.o nodeSubplan.o nodeSubqueryscan.o nodeTidscan.o \
        nodeForeignscan.o nodeWindowAgg.o tstoreReceiver.o tqueue.o spi.o \
        nodeTableFuncscan.o ybcExpr.o ybcFunction.o ybc_fdw.o ybcModifyTable.o \
-       nodeYbBitmapIndexscan.o nodeYbSeqscan.o nodeYbBitmapTablescan.o
+       nodeYbBitmapIndexscan.o nodeYbSeqscan.o nodeYbBitmapTablescan.o \
+       ybOptimizeModifyTable.o
 
 include $(top_srcdir)/src/backend/common.mk

src/postgres/src/backend/executor/execIndexing.c

@@ -279,7 +279,7 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
 {
 	return ExecInsertIndexTuplesOptimized(
 	    slot, tuple, estate, noDupErr, specConflict,
-	    arbiterIndexes, NIL /* no_update_index_list */);
+	    arbiterIndexes);
 }
 
 List *
@@ -288,8 +288,7 @@ ExecInsertIndexTuplesOptimized(TupleTableSlot *slot,
                                EState *estate,
                                bool noDupErr,
                                bool *specConflict,
-                               List *arbiterIndexes,
-                               List *no_update_index_list)
+                               List *arbiterIndexes)
 {
 	List	   *result = NIL;
 	ResultRelInfo *resultRelInfo;
@@ -337,9 +336,10 @@ ExecInsertIndexTuplesOptimized(TupleTableSlot *slot,
 		 * For an update command check if we need to skip index. For that purpose,
 		 * we check if the relid of the index is part of the skip list.
 		 */
-		if (indexRelation == NULL || (no_update_index_list &&
-		    list_member_oid(no_update_index_list, RelationGetRelid(indexRelation))))
-		continue;
+		if (indexRelation == NULL ||
+			list_member_oid(estate->yb_skip_entities.index_list,
+							RelationGetRelid(indexRelation)))
+			continue;
 
 		indexInfo = indexInfoArray[i];
 		Assert(indexInfo->ii_ReadyForInserts ==
@@ -503,14 +503,13 @@ ExecInsertIndexTuplesOptimized(TupleTableSlot *slot,
 void
 ExecDeleteIndexTuples(Datum ybctid, HeapTuple tuple, EState *estate)
 {
-  ExecDeleteIndexTuplesOptimized(ybctid, tuple, estate, NIL /* no_update_index_list */);
+	ExecDeleteIndexTuplesOptimized(ybctid, tuple, estate);
 }
 
 void
 ExecDeleteIndexTuplesOptimized(Datum ybctid,
                                HeapTuple tuple,
-                               EState *estate,
-                               List *no_update_index_list)
+                               EState *estate)
 {
 	ResultRelInfo *resultRelInfo;
 	int			i;
@@ -561,9 +560,10 @@ ExecDeleteIndexTuplesOptimized(Datum ybctid,
 		 * For an update command check if we need to skip index.
 		 * For that purpose, we check if the relid of the index is part of the skip list.
 		 */
-		if (indexRelation == NULL || (no_update_index_list &&
-		    list_member_oid(no_update_index_list, RelationGetRelid(indexRelation))))
-		  continue;
+		if (indexRelation == NULL ||
+			list_member_oid(estate->yb_skip_entities.index_list,
+							RelationGetRelid(indexRelation)))
+			continue;
 
 		/*
 		 * No need to update YugaByte primary key which is intrinic part of

src/postgres/src/backend/executor/execUtils.c

@@ -163,6 +163,8 @@ CreateExecutorState(void)
 	estate->es_jit_flags = 0;
 	estate->es_jit = NULL;
 
+	NodeSetTag(&estate->yb_skip_entities, T_YbSkippableEntities);
+
 	/*
 	 * Return the executor state structure
 	 */

src/postgres/src/backend/executor/nodeModifyTable.c

@@ -72,12 +72,14 @@
 
 /*  YB includes. */
 #include "access/sysattr.h"
+#include "catalog/catalog.h"
 #include "catalog/dependency.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_depend.h"
 #include "catalog/pg_shdepend.h"
 #include "catalog/yb_catalog_version.h"
 #include "executor/ybcModifyTable.h"
+#include "executor/ybOptimizeModifyTable.h"
 #include "parser/parsetree.h"
 #include "pg_yb_utils.h"
 #include "optimizer/ybcplan.h"
@@ -1140,84 +1142,6 @@ ldelete:;
 	return NULL;
 }
 
-/* ----------------------------------------------------------------
- * YBEqualDatums
- *
- * Function compares values of lhs and rhs datums with respect to value type and collation.
- *
- * Returns true in case value of lhs and rhs datums match.
- * ----------------------------------------------------------------
- */
-static bool
-YBEqualDatums(Datum lhs, Datum rhs, Oid atttypid, Oid collation)
-{
-	TypeCacheEntry *typentry = lookup_type_cache(atttypid, TYPECACHE_CMP_PROC_FINFO);
-	if (!OidIsValid(typentry->cmp_proc_finfo.fn_oid))
-		ereport(ERROR,
-		        (errcode(ERRCODE_UNDEFINED_FUNCTION),
-		         errmsg("could not identify a comparison function for type %s",
-		                format_type_be(typentry->type_id))));
-
-	FunctionCallInfoData locfcinfo;
-	InitFunctionCallInfoData(locfcinfo, &typentry->cmp_proc_finfo, 2, collation, NULL, NULL);
-	locfcinfo.arg[0] = lhs;
-	locfcinfo.arg[1] = rhs;
-	locfcinfo.argnull[0] = false;
-	locfcinfo.argnull[1] = false;
-	return DatumGetInt32(FunctionCallInvoke(&locfcinfo)) == 0;
-}
-
-/* ----------------------------------------------------------------
- * YBBuildExtraUpdatedCols
- *
- * Function compares attribute value in oldtuple and newtuple for attributes which are not in the
- * updatedCols set. Returns set of changed attributes or NULL.
- * ----------------------------------------------------------------
- */
-static Bitmapset*
-YBBuildExtraUpdatedCols(Relation rel,
-                        HeapTuple oldtuple,
-                        HeapTuple newtuple,
-                        Bitmapset *updatedCols)
-{
-	if (bms_is_member(InvalidAttrNumber, updatedCols))
-		/* No extra work required in case the whore row is changed */
-		return NULL;
-
-	Bitmapset *result = NULL;
-	AttrNumber firstLowInvalidAttributeNumber = YBGetFirstLowInvalidAttributeNumber(rel);
-	TupleDesc tupleDesc = RelationGetDescr(rel);
-	for (int idx = 0; idx < tupleDesc->natts; ++idx)
-	{
-		FormData_pg_attribute *att_desc = TupleDescAttr(tupleDesc, idx);
-
-		AttrNumber attnum = att_desc->attnum;
-
-		/* Skip virtual (system) and dropped columns */
-		if (!IsRealYBColumn(rel, attnum))
-			continue;
-
-		int bms_idx = attnum - firstLowInvalidAttributeNumber;
-		if (bms_is_member(bms_idx, updatedCols))
-			continue;
-
-		bool old_is_null = false;
-		bool new_is_null = false;
-		Datum old_value = heap_getattr(oldtuple, attnum, tupleDesc, &old_is_null);
-		Datum new_value = heap_getattr(newtuple, attnum, tupleDesc, &new_is_null);
-		if (old_is_null != new_is_null ||
-		    (!new_is_null && !YBEqualDatums(old_value,
-		                                    new_value,
-		                                    att_desc->atttypid,
-		                                    att_desc->attcollation)))
-		{
-			result = bms_add_member(result, bms_idx);
-		}
-	}
-	return result;
-}
-
-
 /*
  * ExecCrossPartitionUpdate --- Move an updated tuple to another partition.
  *
@@ -1547,30 +1471,53 @@ ExecUpdate(ModifyTableState *mtstate,
 
 		bool row_found = false;
 
-		Bitmapset *actualUpdatedCols = rte->updatedCols;
-		Bitmapset *extraUpdatedCols = NULL;
-		if (beforeRowUpdateTriggerFired)
+		/*
+		 * A bitmapset of columns that have been marked as being updated at
+		 * planning time. This set needs to be updated below if either:
+		 * - The update optimization comparing new and old values to identify
+		 *   actually modified columns is enabled.
+		 * - Before row triggers were fired.
+		 */
+		Bitmapset *cols_marked_for_update = bms_copy(rte->updatedCols);
+
+		ModifyTable *plan = (ModifyTable *) mtstate->ps.plan;
+		YbCopySkippableEntities(&estate->yb_skip_entities,
+								plan->yb_skip_entities);
+
+		/*
+		 * If an update is a "single row transaction", then we have already
+		 * confirmed at planning time that it has no secondary indexes or
+		 * triggers or foreign key constraints. Such an update does not
+		 * benefit from optimizations that skip constraint checking or index
+		 * updates.
+		 * While it may seem that a single row, distributed transaction can be
+		 * transformed into a single row, non-distributed transaction, this is
+		 * not the case. It is likely that the row to be updated has been read
+		 * from the storage layer already, thus violating the non-distributed
+		 * transaction semantics.
+		 */
+		if (!estate->yb_es_is_single_row_modify_txn)
 		{
-			/* trigger might have changed tuple */
-			extraUpdatedCols = YBBuildExtraUpdatedCols(
-				resultRelationDesc, oldtuple, tuple, rte->updatedCols);
-			if (extraUpdatedCols)
-			{
-				extraUpdatedCols = bms_add_members(extraUpdatedCols, rte->updatedCols);
-				actualUpdatedCols = extraUpdatedCols;
-			}
+			YbComputeModifiedColumnsAndSkippableEntities(
+				plan, estate, oldtuple, tuple, &cols_marked_for_update,
+				beforeRowUpdateTriggerFired);
 		}
 
-		Bitmapset *primary_key_bms = YBGetTablePrimaryKeyBms(resultRelationDesc);
-		bool is_pk_updated = bms_overlap(primary_key_bms, actualUpdatedCols);
+		/*
+		 * Irrespective of whether the optimization is enabled or not, we have
+		 * to check if the primary key is updated. It could be that the columns
+		 * making up the primary key are not a part of the target list but are
+		 * updated by a before row trigger.
+		 */
+		bool is_pk_updated = YbIsPrimaryKeyUpdated(resultRelationDesc,
+												   cols_marked_for_update);
 
 		/*
 		 * TODO(alex): It probably makes more sense to pass a
 		 *             transformed slot instead of a plan slot? Note though
 		 *             that it can have tuple materialized already.
 		 */
 
-		ModifyTable *plan = (ModifyTable *) mtstate->ps.plan;
 		if (is_pk_updated)
 		{
 			slot->tts_tuple->t_ybctid = YBCGetYBTupleIdFromSlot(planSlot);
@@ -1588,11 +1535,10 @@ ExecUpdate(ModifyTableState *mtstate,
 										 mtstate->yb_fetch_target_tuple,
 										 estate->yb_es_is_single_row_modify_txn
 										 		? YB_SINGLE_SHARD_TRANSACTION : YB_TRANSACTIONAL,
-										 actualUpdatedCols,
+										 cols_marked_for_update,
 										 canSetTag);
 		}
 
-		bms_free(extraUpdatedCols);
 		if (!row_found)
 		{
 			/*
@@ -1610,14 +1556,13 @@ ExecUpdate(ModifyTableState *mtstate,
 			mtstate->yb_fetch_target_tuple)
 		{
 			Datum	ybctid = YBCGetYBTupleIdFromSlot(planSlot);
-			List *no_update_index_list = ((ModifyTable *)mtstate->ps.plan)->no_update_index_list;
 
 			/* Delete index entries of the old tuple */
-			ExecDeleteIndexTuplesOptimized(ybctid, oldtuple, estate, no_update_index_list);
+			ExecDeleteIndexTuplesOptimized(ybctid, oldtuple, estate);
 
 			/* Insert new index entries for tuple */
 			recheckIndexes = ExecInsertIndexTuplesOptimized(
-			    slot, tuple, estate, false, NULL, NIL, no_update_index_list);
+				slot, tuple, estate, false, NULL, NIL);
 		}
 	}
 	else
@@ -1833,6 +1778,9 @@ lreplace:;
 							mtstate->mt_oc_transition_capture :
 							mtstate->mt_transition_capture);
 	}
+
+	YbClearSkippableEntities(&estate->yb_skip_entities);
+
 	list_free(recheckIndexes);
 
 	/*
@@ -2519,6 +2467,32 @@ tupconv_map_for_subplan(ModifyTableState *mtstate, int whichplan)
 	}
 }
 
+/*
+ * A tuple of a result relation has the "wholerow" junk attribute set at
+ * planning time if attributes other than the tuple's YBCTID need to be read at
+ * the time of execution. Examples of this include UPDATEs and DELETEs on
+ * relations that have secondary indexes -- the columns referenced by the index
+ * need to be read in order to update or delete them.
+ * Another example is when "update optimizations" are enabled. The optimizations
+ * may read the values of multiple attributes to decide whether or not to update
+ * them.
+ * "wholerow" junk attribute is not applicable to INSERTs as all the information
+ * needed for inserting the tuple is supplied in the query.
+ */
+static bool
+YBCHasWholeRowJunkAttr(ResultRelInfo *resultRelInfo, CmdType operation)
+{
+	if (operation == CMD_UPDATE && YbIsUpdateOptimizationEnabled())
+		return true;
+
+	if (operation == CMD_UPDATE || operation == CMD_DELETE)
+		return YBCRelInfoHasSecondaryIndices(resultRelInfo) ||
+			   YBRelHasOldRowTriggers(resultRelInfo->ri_RelationDesc,
+									  operation);
+
+	return false;
+}
+
 /* ----------------------------------------------------------------
  *	   ExecModifyTable
  *
@@ -2697,9 +2671,7 @@ ExecModifyTable(PlanState *pstate)
 				 *    trigger execution.
 				 */
 				if (IsYBRelation(resultRelInfo->ri_RelationDesc) &&
-					(YBCRelInfoHasSecondaryIndices(resultRelInfo) ||
-					YBRelHasOldRowTriggers(resultRelInfo->ri_RelationDesc,
-					                       operation)))
+					YBCHasWholeRowJunkAttr(resultRelInfo, operation))
 				{
 					resno = ExecFindJunkAttribute(junkfilter, "wholerow");
 					datum = ExecGetJunkAttribute(slot, resno, &isNull);