[#18822] YSQL: Write out unmodified cols to main table when update op…

…timizations are enabled

Summary:
D34040 introduced a framework to skip redundant secondary index updates and foreign key checks.
As a part of the implementation, this revision skipped writing out unmodified columns to the main table.
For correctness reasons, skipping writes of specific columns to the main table requires the acquisition of row-level locks.
(In the event that no columns are modified, one is still required to acquire a row lock on the affected row)

This created a dependency between the update optimization and the row locking feature.
The latter is controlled by the autoflag `ysql_skip_row_lock_for_update`.
This revision seeks to remove this dependency by continuing to write out unmodified columns to the main table.

There is one notable exception to this behavior: unmodified columns that are a part of the primary key.
If the value of the primary key remains unmodified, its columns are not written out to the main table as this would require an extra round trip to the storage layer.
Jira: DB-7701

Test Plan:
Run the following tests and ensure that there are no regressions:
```
./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressUpdateOptimized#schedule'
```

Reviewers: amartsinchyk

Reviewed By: amartsinchyk

Subscribers: smishra, yql

Tags: #jenkins-ready

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

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

@@ -1473,11 +1473,17 @@ ExecUpdate(ModifyTableState *mtstate,
 		bool row_found = false;
 
 		/*
-		 * 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.
+		 * A bitmapset of columns whose values are written to the main table.
+		 * This is initialized to the set of columns marked for update at
+		 * planning time. This set is updated as follows:
+		 * - Columns modified by before row triggers are added.
+		 * - Primary key columns in the setlist that are unmodified are removed.
+		 *
+		 * Maintaining this bitmapset allows us to continue writing out
+		 * unmodified columns to the main table as a safety guardrail, while
+		 * selectively skipping index updates and constraint checks.
+		 * This guardrail may be removed in the future. This also helps avoid
+		 * having a dependency on row locking.
 		 */
 		Bitmapset *cols_marked_for_update = bms_copy(rte->updatedCols);
 
@@ -1565,6 +1571,8 @@ ExecUpdate(ModifyTableState *mtstate,
 			recheckIndexes = ExecInsertIndexTuplesOptimized(
 				slot, tuple, estate, false, NULL, NIL);
 		}
+
+		bms_free(cols_marked_for_update);
 	}
 	else
 	{

src/postgres/src/backend/executor/ybOptimizeModifyTable.c

@@ -468,7 +468,7 @@ YbComputeModifiedEntities(ResultRelInfo *resultRelInfo, HeapTuple oldtuple,
  * skipped as a result of unmodified columns.
  * A list of modified columns (updated_cols) is required:
  * - To construct the payload of updated values to be send to the storage layer.
- * - To determine if column-specific after-row triggers are elgible to fire.
+ * - To determine if column-specific after-row triggers are eligible to fire.
  * ----------------------------------------------------------------
  */
 void
@@ -532,9 +532,21 @@ YbComputeModifiedColumnsAndSkippableEntities(
 								   plan->onConflictAction == ONCONFLICT_UPDATE));
 	}
 
-	*updated_cols = bms_del_members(*updated_cols, unmodified_cols);
 	*updated_cols = bms_add_members(*updated_cols, modified_cols);
 
+	/*
+	 * Exclude unmodified primary key columns from the set of main table columns
+	 * to be updated. This potentially prevents an extra RPC to the main table.
+	 * Do not bother if the any part of the primary key is modified.
+	 */
+	if (!bms_overlap(YBGetTablePrimaryKeyBms(rel), modified_cols))
+	{
+		Bitmapset *unmodified_pkcols =
+			bms_intersect(YBGetTablePrimaryKeyBms(rel), unmodified_cols);
+		*updated_cols = bms_del_members(*updated_cols, unmodified_pkcols);
+		bms_free(unmodified_pkcols);
+	}
+
 	YbLogInspectedColumns(rel, *updated_cols, modified_cols, unmodified_cols);
 
 	bms_free(unmodified_cols);

src/postgres/src/backend/utils/misc/pg_yb_utils.c

@@ -4890,8 +4890,7 @@ bool
 YbIsUpdateOptimizationEnabled()
 {
 	/* TODO(kramanathan): Placeholder until a flag strategy is agreed upon */
-	return (!YBCIsEnvVarTrue("FLAGS_ysql_skip_row_lock_for_update")) &&
-		   yb_update_optimization_options.num_cols_to_compare > 0 &&
+	return yb_update_optimization_options.num_cols_to_compare > 0 &&
 		   yb_update_optimization_options.max_cols_size_to_compare > 0;
 }