From d9a860f52ac66a7a6c0cc35ce3356cad882b321a Mon Sep 17 00:00:00 2001 From: Mihnea Iancu Date: Thu, 2 Apr 2020 00:05:03 -0700 Subject: [PATCH] [YSQL] Single row update overwrites unmodified columns with default values to null #4127 Summary: Fix a regression for single-row updates and columns with default values introduced commit 7e222b42902b5d73067e96ce58a26e679f11a618. 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/backend/executor/ybcModifyTable.c | 10 +- .../regress/expected/yb_dml_single_row.out | 177 ++++++++++++++++++ .../test/regress/sql/yb_dml_single_row.sql | 67 +++++++ 3 files changed, 251 insertions(+), 3 deletions(-) diff --git a/src/postgres/src/backend/executor/ybcModifyTable.c b/src/postgres/src/backend/executor/ybcModifyTable.c index 312d79a81086..0939dac1a559 100644 --- a/src/postgres/src/backend/executor/ybcModifyTable.c +++ b/src/postgres/src/backend/executor/ybcModifyTable.c @@ -688,7 +688,6 @@ 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; @@ -696,9 +695,14 @@ bool YBCExecuteUpdate(Relation rel, 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. */ diff --git a/src/postgres/src/test/regress/expected/yb_dml_single_row.out b/src/postgres/src/test/regress/expected/yb_dml_single_row.out index 97fe8e14312b..c8ed4f755dcb 100644 --- a/src/postgres/src/test/regress/expected/yb_dml_single_row.out +++ b/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) diff --git a/src/postgres/src/test/regress/sql/yb_dml_single_row.sql b/src/postgres/src/test/regress/sql/yb_dml_single_row.sql index edbaefb11ba8..2264bfa5c299 100644 --- a/src/postgres/src/test/regress/sql/yb_dml_single_row.sql +++ b/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;