[YSQL] Single row update overwrites unmodified columns with default v…

…alues to null #4127

Summary:
Fix a regression for single-row updates and columns with default values
introduced commit 7e222b4.

Default values for columns need to be handled for INSERTs (i.e. to be used
in case there is no user-provided value).
However, UPDATEs should not modify columns that were explicitly set,
regardless of whether a default is set for that column.
Therefore, there is no need to distinguish columns with default for the
UPDATE case, so revert the change that did that.
Instead, we need to distinguish between single-row and non-single-row
execution for update, because non-single-row execution can have triggers
modifying columns values (set or defaults).

Test Plan: TestPgRegressDml (yb_dml_single_row), existing TestPgRegressBetaFeatures (yb_pg_triggers).

Reviewers: neha, sudheer

Reviewed By: neha

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D8225

src/postgres/src/backend/executor/ybcModifyTable.c

@@ -688,17 +688,21 @@ bool YBCExecuteUpdate(Relation rel,
 		FormData_pg_attribute *att_desc = TupleDescAttr(tupleDesc, idx);
 
 		AttrNumber attnum = att_desc->attnum;
-		bool has_default = att_desc->atthasdef;
 		int32_t type_id = att_desc->atttypid;
 		int32_t type_mod = att_desc->atttypmod;
 
 		/* Skip virtual (system) and dropped columns */
 		if (!IsRealYBColumn(rel, attnum))
 			continue;
 
-		/* Skip unmodified columns */
+		/*
+		 * Skip unmodified columns if possible.
+		 * Note: we only do this for the single-row case, as otherwise there
+		 * might be triggers that modify the heap tuple to set (other) columns
+		 * (e.g. using the SPI module functions).
+		 */
 		int bms_idx = attnum - YBGetFirstLowInvalidAttributeNumber(rel);
-		if (!whole_row && !bms_is_member(bms_idx, updatedCols) && !has_default)
+		if (isSingleRow && !whole_row && !bms_is_member(bms_idx, updatedCols))
 			continue;
 
 		/* Assign this attr's value, handle expression pushdown if needed. */

src/postgres/src/test/regress/expected/yb_dml_single_row.out

@@ -1848,3 +1848,180 @@ SELECT * FROM single_row_col_order ORDER BY d, b;
 ---+---+---+---+---
  2 | 3 | 4 | 5 | 6
 (1 row)
+
+--
+-- Test single-row with default values
+--
+CREATE TABLE single_row_default_col(k int PRIMARY KEY, a int default 10, b int default 20 not null, c int);
+------------------
+-- Test Planning
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET a = 3 WHERE k = 2;
+            QUERY PLAN
+----------------------------------
+ Update on single_row_default_col
+   ->  Result
+(2 rows)
+
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET b = 3 WHERE k = 2;
+            QUERY PLAN
+----------------------------------
+ Update on single_row_default_col
+   ->  Result
+(2 rows)
+
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET a = 3, c = 4 WHERE k = 2;
+            QUERY PLAN
+----------------------------------
+ Update on single_row_default_col
+   ->  Result
+(2 rows)
+
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET b = NULL, c = 5 WHERE k = 3;
+            QUERY PLAN
+----------------------------------
+ Update on single_row_default_col
+   ->  Result
+(2 rows)
+
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET a = 3, b = 3, c = 3 WHERE k = 2;
+            QUERY PLAN
+----------------------------------
+ Update on single_row_default_col
+   ->  Result
+(2 rows)
+
+------------------
+-- Test Execution
+-- Insert should use defaults for missing columns.
+INSERT INTO single_row_default_col(k, a, b, c) VALUES (1, 1, 1, 1);
+INSERT INTO single_row_default_col(k, c) VALUES (2, 2);
+INSERT INTO single_row_default_col(k, a, c) VALUES (3, NULL, 3);
+INSERT INTO single_row_default_col(k, a, c) VALUES (4, NULL, 4);
+-- Setting b to null should not be allowed.
+INSERT INTO single_row_default_col(k, a, b, c) VALUES (5, 5, NULL, 5);
+ERROR:  null value in column "b" violates not-null constraint
+DETAIL:  Failing row contains (5, 5, null, 5).
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a  | b  | c
+---+----+----+---
+ 1 |  1 |  1 | 1
+ 2 | 10 | 20 | 2
+ 3 |    | 20 | 3
+ 4 |    | 20 | 4
+(4 rows)
+
+-- Updates should not modify the existing (non-updated) values.
+UPDATE single_row_default_col SET b = 3 WHERE k = 2;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a  | b  | c
+---+----+----+---
+ 1 |  1 |  1 | 1
+ 2 | 10 |  3 | 2
+ 3 |    | 20 | 3
+ 4 |    | 20 | 4
+(4 rows)
+
+UPDATE single_row_default_col SET a = 3, c = 4 WHERE k = 2;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a | b  | c
+---+---+----+---
+ 1 | 1 |  1 | 1
+ 2 | 3 |  3 | 4
+ 3 |   | 20 | 3
+ 4 |   | 20 | 4
+(4 rows)
+
+-- a should stay null (because it was explicitly set).
+UPDATE single_row_default_col SET b = 4, c = 5 WHERE k = 3;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a | b  | c
+---+---+----+---
+ 1 | 1 |  1 | 1
+ 2 | 3 |  3 | 4
+ 3 |   |  4 | 5
+ 4 |   | 20 | 4
+(4 rows)
+
+UPDATE single_row_default_col SET a = 4 WHERE k = 3;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a | b  | c
+---+---+----+---
+ 1 | 1 |  1 | 1
+ 2 | 3 |  3 | 4
+ 3 | 4 |  4 | 5
+ 4 |   | 20 | 4
+(4 rows)
+
+-- Setting b to null should not be allowed.
+UPDATE single_row_default_col SET b = NULL, c = 5 WHERE k = 3;
+ERROR:  null value in column "b" violates not-null constraint
+DETAIL:  Failing row contains (3, null, null, 5).
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a | b  | c
+---+---+----+---
+ 1 | 1 |  1 | 1
+ 2 | 3 |  3 | 4
+ 3 | 4 |  4 | 5
+ 4 |   | 20 | 4
+(4 rows)
+
+ALTER TABLE single_row_default_col ALTER COLUMN a SET DEFAULT 30;
+-- Insert should use the new default.
+INSERT INTO single_row_default_col(k, c) VALUES (5, 5);
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a  | b  | c
+---+----+----+---
+ 1 |  1 |  1 | 1
+ 2 |  3 |  3 | 4
+ 3 |  4 |  4 | 5
+ 4 |    | 20 | 4
+ 5 | 30 | 20 | 5
+(5 rows)
+
+-- Updates should not modify the existing (non-updated) values.
+UPDATE single_row_default_col SET a = 4 WHERE k = 4;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a  | b  | c
+---+----+----+---
+ 1 |  1 |  1 | 1
+ 2 |  3 |  3 | 4
+ 3 |  4 |  4 | 5
+ 4 |  4 | 20 | 4
+ 5 | 30 | 20 | 5
+(5 rows)
+
+UPDATE single_row_default_col SET c = 6, b = 5 WHERE k = 5;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a  | b  | c
+---+----+----+---
+ 1 |  1 |  1 | 1
+ 2 |  3 |  3 | 4
+ 3 |  4 |  4 | 5
+ 4 |  4 | 20 | 4
+ 5 | 30 |  5 | 6
+(5 rows)
+
+UPDATE single_row_default_col SET c = 7, b = 7, a = 7 WHERE k = 5;
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a | b  | c
+---+---+----+---
+ 1 | 1 |  1 | 1
+ 2 | 3 |  3 | 4
+ 3 | 4 |  4 | 5
+ 4 | 4 | 20 | 4
+ 5 | 7 |  7 | 7
+(5 rows)
+
+-- Setting b to null should not be allowed.
+UPDATE single_row_default_col SET b = NULL, c = 5 WHERE k = 3;
+ERROR:  null value in column "b" violates not-null constraint
+DETAIL:  Failing row contains (3, null, null, 5).
+SELECT * FROM single_row_default_col ORDER BY k;
+ k | a | b  | c
+---+---+----+---
+ 1 | 1 |  1 | 1
+ 2 | 3 |  3 | 4
+ 3 | 4 |  4 | 5
+ 4 | 4 | 20 | 4
+ 5 | 7 |  7 | 7
+(5 rows)

src/postgres/src/test/regress/sql/yb_dml_single_row.sql

@@ -552,3 +552,70 @@ SELECT * FROM single_row_col_order ORDER BY d, b;
 
 DELETE FROM single_row_col_order WHERE b = 2 and d = 4;
 SELECT * FROM single_row_col_order ORDER BY d, b;
+
+--
+-- Test single-row with default values
+--
+
+CREATE TABLE single_row_default_col(k int PRIMARY KEY, a int default 10, b int default 20 not null, c int);
+
+------------------
+-- Test Planning
+
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET a = 3 WHERE k = 2;
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET b = 3 WHERE k = 2;
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET a = 3, c = 4 WHERE k = 2;
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET b = NULL, c = 5 WHERE k = 3;
+EXPLAIN (COSTS FALSE) UPDATE single_row_default_col SET a = 3, b = 3, c = 3 WHERE k = 2;
+
+------------------
+-- Test Execution
+
+-- Insert should use defaults for missing columns.
+INSERT INTO single_row_default_col(k, a, b, c) VALUES (1, 1, 1, 1);
+INSERT INTO single_row_default_col(k, c) VALUES (2, 2);
+INSERT INTO single_row_default_col(k, a, c) VALUES (3, NULL, 3);
+INSERT INTO single_row_default_col(k, a, c) VALUES (4, NULL, 4);
+
+-- Setting b to null should not be allowed.
+INSERT INTO single_row_default_col(k, a, b, c) VALUES (5, 5, NULL, 5);
+
+SELECT * FROM single_row_default_col ORDER BY k;
+
+-- Updates should not modify the existing (non-updated) values.
+UPDATE single_row_default_col SET b = 3 WHERE k = 2;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+UPDATE single_row_default_col SET a = 3, c = 4 WHERE k = 2;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+-- a should stay null (because it was explicitly set).
+UPDATE single_row_default_col SET b = 4, c = 5 WHERE k = 3;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+UPDATE single_row_default_col SET a = 4 WHERE k = 3;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+-- Setting b to null should not be allowed.
+UPDATE single_row_default_col SET b = NULL, c = 5 WHERE k = 3;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+ALTER TABLE single_row_default_col ALTER COLUMN a SET DEFAULT 30;
+
+-- Insert should use the new default.
+INSERT INTO single_row_default_col(k, c) VALUES (5, 5);
+SELECT * FROM single_row_default_col ORDER BY k;
+
+-- Updates should not modify the existing (non-updated) values.
+UPDATE single_row_default_col SET a = 4 WHERE k = 4;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+UPDATE single_row_default_col SET c = 6, b = 5 WHERE k = 5;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+UPDATE single_row_default_col SET c = 7, b = 7, a = 7 WHERE k = 5;
+SELECT * FROM single_row_default_col ORDER BY k;
+
+-- Setting b to null should not be allowed.
+UPDATE single_row_default_col SET b = NULL, c = 5 WHERE k = 3;
+SELECT * FROM single_row_default_col ORDER BY k;