Skip to content

Commit

Permalink
Merge #133328
Browse files Browse the repository at this point in the history 
133328: sql: fix trigger interaction with computed columns r=mgartner a=DrewKimball

This commit makes the following changes to the way row-level BEFORE triggers interact with computed columns:
* BEFORE triggers now observe NULL values in place of computed columns.
* Modifications to computed columns by BEFORE triggers are ignored.
* Computed columns are re-computed after row-level BEFORE triggers execute.

These changes make the behavior consistent with that of Postgres.

Fixes #132979

Release note: None

Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
  • Loading branch information
@DrewKimball
craig[bot] and DrewKimball committed Nov 6, 2024
2 parents 72fd7e2 + 43d6d62 commit f5f5cbc
Show file tree
Hide file tree
Showing 3 changed files with 366 additions and 5 deletions.
234 changes: 234 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/triggers
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2845,6 +2845,240 @@ DROP FUNCTION g;
statement ok
DELETE FROM xy WHERE True;

# ==============================================================================
# Test row-level trigger interaction with computed columns.
# ==============================================================================

# BEFORE triggers do not observe the values of computed columns. Instead, they
# see NULLs. In addition, while a BEFORE trigger can modify a computed column,
# the changed value is ignored. In contrast, the computed column *does* show
# the effect of modifications to the columns it depends on.
subtest before_computed_columns

statement ok
CREATE TABLE computed (a INT, b INT, c INT AS (a + b) STORED, d INT AS (a - b) VIRTUAL);

statement ok
CREATE FUNCTION g() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
BEGIN
RAISE NOTICE 'g()';
RAISE NOTICE 'NEW: %', NEW;
NEW.c = 12345;
NEW.d = 67890;
RAISE NOTICE 'NEW after change: %', NEW;
RETURN NEW;
END
$$;

statement ok
CREATE TRIGGER b_foo BEFORE INSERT OR UPDATE ON computed FOR EACH ROW EXECUTE FUNCTION g();

# BEFORE triggers do not observe the values of computed columns. Instead, they
# see NULLs. In addition, while a BEFORE trigger can modify a computed column,
# the changed value is ignored.
query T noticetrace
INSERT INTO computed VALUES (1, 2);
----
NOTICE: g()
NOTICE: NEW: (1,2,,)
NOTICE: NEW after change: (1,2,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
1 2 3 -1

query T noticetrace
UPDATE computed SET a = 10, b = 20 WHERE a = 1;
----
NOTICE: g()
NOTICE: NEW: (10,20,,)
NOTICE: NEW after change: (10,20,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10

# The computed column *does* show the effect of modifications to the columns it
# depends on.
statement ok
CREATE FUNCTION h() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
BEGIN
RAISE NOTICE 'h()';
RAISE NOTICE 'NEW: %', NEW;
NEW.a = (NEW).a + 100;
NEW.b = (NEW).b + 200;
RAISE NOTICE 'NEW after change: %', NEW;
RETURN NEW;
END
$$;

# Create triggers that fire before and after the previous one.
statement ok
CREATE TRIGGER a_foo BEFORE INSERT OR UPDATE ON computed FOR EACH ROW EXECUTE FUNCTION h();

statement ok
CREATE TRIGGER c_foo BEFORE INSERT OR UPDATE ON computed FOR EACH ROW EXECUTE FUNCTION h();

query T noticetrace
INSERT INTO computed VALUES (3, 4);
----
NOTICE: h()
NOTICE: NEW: (3,4,,)
NOTICE: NEW after change: (103,204,,)
NOTICE: g()
NOTICE: NEW: (103,204,,)
NOTICE: NEW after change: (103,204,12345,67890)
NOTICE: h()
NOTICE: NEW: (103,204,12345,67890)
NOTICE: NEW after change: (203,404,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10
203 404 607 -201

query T noticetrace
UPDATE computed SET a = 30, b = 40 WHERE a = 203;
----
NOTICE: h()
NOTICE: NEW: (30,40,,)
NOTICE: NEW after change: (130,240,,)
NOTICE: g()
NOTICE: NEW: (130,240,,)
NOTICE: NEW after change: (130,240,12345,67890)
NOTICE: h()
NOTICE: NEW: (130,240,12345,67890)
NOTICE: NEW after change: (230,440,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10
230 440 670 -210

# AFTER triggers observe the values of computed columns. They can also modify
# computed columns, but the updated value is ignored (in fact, any modification
# is ignored for AFTER triggers).
subtest after_computed_columns

statement ok
CREATE TRIGGER trig AFTER INSERT OR UPDATE ON computed FOR EACH ROW EXECUTE FUNCTION g();

query T noticetrace
INSERT INTO computed VALUES (5, 6);
----
NOTICE: h()
NOTICE: NEW: (5,6,,)
NOTICE: NEW after change: (105,206,,)
NOTICE: g()
NOTICE: NEW: (105,206,,)
NOTICE: NEW after change: (105,206,12345,67890)
NOTICE: h()
NOTICE: NEW: (105,206,12345,67890)
NOTICE: NEW after change: (205,406,12345,67890)
NOTICE: g()
NOTICE: NEW: (205,406,611,-201)
NOTICE: NEW after change: (205,406,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10
230 440 670 -210
205 406 611 -201

query T noticetrace
UPDATE computed SET a = 50, b = 60 WHERE a = 230;
----
NOTICE: h()
NOTICE: NEW: (50,60,,)
NOTICE: NEW after change: (150,260,,)
NOTICE: g()
NOTICE: NEW: (150,260,,)
NOTICE: NEW after change: (150,260,12345,67890)
NOTICE: h()
NOTICE: NEW: (150,260,12345,67890)
NOTICE: NEW after change: (250,460,12345,67890)
NOTICE: g()
NOTICE: NEW: (250,460,710,-210)
NOTICE: NEW after change: (250,460,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10
250 460 710 -210
205 406 611 -201

statement ok
DROP TRIGGER trig ON computed;

statement ok
DROP TRIGGER a_foo ON computed;

statement ok
DROP TRIGGER b_foo ON computed;

statement ok
DROP TRIGGER c_foo ON computed;

# The WHEN clause for a BEFORE trigger should observe NULL values for computed
# columns. For an AFTER trigger, the WHEN clause should observe the computed
# values.
subtest when_computed_columns

statement ok
CREATE TRIGGER trig BEFORE INSERT OR UPDATE ON computed
FOR EACH ROW WHEN ((NEW).c IS NULL AND (NEW).d IS NULL) EXECUTE FUNCTION g();

statement ok
CREATE TRIGGER trig_after AFTER INSERT OR UPDATE ON computed
FOR EACH ROW WHEN ((NEW).c IS NOT NULL AND (NEW).d IS NOT NULL) EXECUTE FUNCTION g();

query T noticetrace
INSERT INTO computed VALUES (7, 8);
----
NOTICE: g()
NOTICE: NEW: (7,8,,)
NOTICE: NEW after change: (7,8,12345,67890)
NOTICE: g()
NOTICE: NEW: (7,8,15,-1)
NOTICE: NEW after change: (7,8,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10
250 460 710 -210
205 406 611 -201
7 8 15 -1

query T noticetrace
UPDATE computed SET a = 70, b = 80 WHERE a = 7;
----
NOTICE: g()
NOTICE: NEW: (70,80,,)
NOTICE: NEW after change: (70,80,12345,67890)
NOTICE: g()
NOTICE: NEW: (70,80,150,-10)
NOTICE: NEW after change: (70,80,12345,67890)

query IIII rowsort
SELECT * FROM computed;
----
10 20 30 -10
250 460 710 -210
205 406 611 -201
70 80 150 -10

statement ok
DROP TABLE computed;
DROP FUNCTION g;
DROP FUNCTION h;

# ==============================================================================
# Test unsupported syntax.
# ==============================================================================
Expand Down
98 changes: 97 additions & 1 deletion pkg/sql/opt/optbuilder/testdata/trigger
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ exec-ddl
CREATE TABLE child (k INT PRIMARY KEY, x INT REFERENCES xy(x) ON UPDATE CASCADE ON DELETE CASCADE);
----

exec-ddl
CREATE TABLE computed (k INT PRIMARY KEY, v INT AS (k + 1) STORED, w INT AS (k + 2) VIRTUAL);
----

exec-ddl
CREATE FUNCTION f() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
BEGIN
Expand Down Expand Up @@ -539,6 +543,98 @@ root
└── filters
└── x:76 = xy.x:77

# Show interaction with computed columns.
exec-ddl
CREATE TRIGGER tr BEFORE INSERT OR UPDATE ON computed FOR EACH ROW
WHEN ((NEW).v IS NULL AND (NEW).w IS NULL) EXECUTE FUNCTION f();
----

norm format=(hide-all,show-columns)
INSERT INTO computed (k) VALUES (1);
----
insert computed
├── columns: <none>
├── insert-mapping:
│ ├── k_new:24 => k:1
│ ├── v_comp:25 => v:2
│ └── w_comp:26 => w:3
└── project
├── columns: v_comp:25 w_comp:26 column1:6 v_comp:7 w_comp:8 new:9 f:23 k_new:24
├── project
│ ├── columns: k_new:24 column1:6 v_comp:7 w_comp:8 new:9 f:23
│ ├── barrier
│ │ ├── columns: column1:6 v_comp:7 w_comp:8 new:9 f:23
│ │ └── select
│ │ ├── columns: column1:6 v_comp:7 w_comp:8 new:9 f:23
│ │ ├── project
│ │ │ ├── columns: f:23 column1:6 v_comp:7 w_comp:8 new:9
│ │ │ ├── barrier
│ │ │ │ ├── columns: column1:6 v_comp:7 w_comp:8 new:9
│ │ │ │ └── values
│ │ │ │ ├── columns: column1:6 v_comp:7 w_comp:8 new:9
│ │ │ │ └── (1, 2, 3, ((1, NULL, NULL) AS k, v, w))
│ │ │ └── projections
│ │ │ └── CASE WHEN ((new:9).v IS NULL) AND ((new:9).w IS NULL) THEN f(new:9, NULL, 'tr', 'BEFORE', 'ROW', 'INSERT', 55, 'computed', 'computed', 'public', 0, ARRAY[]) ELSE new:9 END [as=f:23]
│ │ └── filters
│ │ └── f:23 IS DISTINCT FROM NULL
│ └── projections
│ └── (f:23).k [as=k_new:24]
└── projections
├── k_new:24 + 1 [as=v_comp:25]
└── k_new:24 + 2 [as=w_comp:26]

norm format=(hide-all,show-columns)
UPDATE computed SET k = 2 WHERE k = 1;
----
update computed
├── columns: <none>
├── fetch columns: k:6 v:7 w:8
├── update-mapping:
│ ├── k_new:30 => k:1
│ ├── v_comp:31 => v:2
│ └── w_comp:32 => w:3
└── project
├── columns: v_comp:31 w_comp:32 k:6 v:7 w:8 k_new:30
├── project
│ ├── columns: k_new:30 k:6 v:7 w:8
│ ├── barrier
│ │ ├── columns: k:6 v:7 w:8 crdb_internal_mvcc_timestamp:9 tableoid:10 k_new:11 v_comp:12 w_comp:13 old:14 new:15 f:29
│ │ └── select
│ │ ├── columns: k:6 v:7 w:8 crdb_internal_mvcc_timestamp:9 tableoid:10 k_new:11 v_comp:12 w_comp:13 old:14 new:15 f:29
│ │ ├── project
│ │ │ ├── columns: f:29 k:6 v:7 w:8 crdb_internal_mvcc_timestamp:9 tableoid:10 k_new:11 v_comp:12 w_comp:13 old:14 new:15
│ │ │ ├── barrier
│ │ │ │ ├── columns: k:6 v:7 w:8 crdb_internal_mvcc_timestamp:9 tableoid:10 k_new:11 v_comp:12 w_comp:13 old:14 new:15
│ │ │ │ └── project
│ │ │ │ ├── columns: new:15 old:14 v_comp:12 w_comp:13 k_new:11 w:8 k:6 v:7 crdb_internal_mvcc_timestamp:9 tableoid:10
│ │ │ │ ├── select
│ │ │ │ │ ├── columns: k:6 v:7 crdb_internal_mvcc_timestamp:9 tableoid:10
│ │ │ │ │ ├── scan computed
│ │ │ │ │ │ ├── columns: k:6 v:7 crdb_internal_mvcc_timestamp:9 tableoid:10
│ │ │ │ │ │ └── computed column expressions
│ │ │ │ │ │ ├── v:7
│ │ │ │ │ │ │ └── k:6 + 1
│ │ │ │ │ │ └── w:8
│ │ │ │ │ │ └── k:6 + 2
│ │ │ │ │ └── filters
│ │ │ │ │ └── k:6 = 1
│ │ │ │ └── projections
│ │ │ │ ├── ((2, NULL, NULL) AS k, v, w) [as=new:15]
│ │ │ │ ├── ((k:6, CAST(NULL AS INT8), CAST(NULL AS INT8)) AS k, v, w) [as=old:14]
│ │ │ │ ├── 3 [as=v_comp:12]
│ │ │ │ ├── 4 [as=w_comp:13]
│ │ │ │ ├── 2 [as=k_new:11]
│ │ │ │ └── k:6 + 2 [as=w:8]
│ │ │ └── projections
│ │ │ └── CASE WHEN ((new:15).v IS NULL) AND ((new:15).w IS NULL) THEN f(new:15, old:14, 'tr', 'BEFORE', 'ROW', 'UPDATE', 55, 'computed', 'computed', 'public', 0, ARRAY[]) ELSE new:15 END [as=f:29]
│ │ └── filters
│ │ └── f:29 IS DISTINCT FROM NULL
│ └── projections
│ └── (f:29).k [as=k_new:30]
└── projections
├── k_new:30 + 1 [as=v_comp:31]
└── k_new:30 + 2 [as=w_comp:32]

# ------------------------------------------------------------------------------
# Row-level AFTER triggers.
# ------------------------------------------------------------------------------
Expand Down Expand Up @@ -942,7 +1038,7 @@ insert t133329
│ │ │ │ └── projections
│ │ │ │ └── ((column1, column2) AS k, a)
│ │ │ └── projections
│ │ │ └── f(new, NULL, 'tr', 'BEFORE', 'ROW', 'INSERT', 56, 't133329', 't133329', 'public', 0, ARRAY[])
│ │ │ └── f(new, NULL, 'tr', 'BEFORE', 'ROW', 'INSERT', 57, 't133329', 't133329', 'public', 0, ARRAY[])
│ │ └── filters
│ │ └── f IS DISTINCT FROM NULL
│ └── projections
Expand Down
Loading

0 comments on commit f5f5cbc

Please sign in to comment.