Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: incorrect handling for computed columns in row-level BEFORE triggers #132979

Closed
DrewKimball opened this issue Oct 19, 2024 · 1 comment · Fixed by #133328
Closed

sql: incorrect handling for computed columns in row-level BEFORE triggers #132979

DrewKimball opened this issue Oct 19, 2024 · 1 comment · Fixed by #133328
Assignees
Labels
A-sql-trigger Triggers and Trigger Functions backport-24.3.x Flags PRs that need to be backported to 24.3 branch-master Failures and bugs on the master branch. branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-queries SQL Queries Team

Comments

@DrewKimball
Copy link
Collaborator

DrewKimball commented Oct 19, 2024

Currently, row-level BEFORE triggers are projected after computed columns. This means that the trigger will be able to "see" and directly modify the computed values, and that modifications made by the trigger won't be visible in the computed column.

Contrast this with Postgres:

Generated columns are, conceptually, updated after BEFORE triggers have run. Therefore, changes made to base columns in a >BEFORE trigger will be reflected in generated columns. But conversely, it is not allowed to access generated columns in BEFORE >triggers.

Example with direct modification of the computed column.

postgres=# create table t (x int primary key, y int generated always as (x * 10) stored);
CREATE TABLE
postgres=# create function f() returns trigger language plpgsql as $$ begin raise notice '%', new; new.y := -1; return new; end $$;
CREATE FUNCTION
postgres=# create trigger trig before insert on t for each row execute function f();
CREATE TRIGGER
postgres=# insert into t values (1), (2);
NOTICE:  (1,)
NOTICE:  (2,)
INSERT 0 2
postgres=# select * from t;
 x | y
---+----
 1 | 10
 2 | 20
(2 rows)

Example with modification of a referenced column.

postgres=# create function f() returns trigger language plpgsql as $$ begin raise notice '%', new; new.x := new.x + 10; return new; end $$;
CREATE FUNCTION
postgres=# create trigger trig before insert on t for each row execute function f();
CREATE TRIGGER
postgres=# insert into t values (1), (2);
NOTICE:  (1,)
NOTICE:  (2,)
INSERT 0 2
postgres=# select * from t;
 x  |  y
----+-----
 11 | 110
 12 | 120
(2 rows)

Jira issue: CRDB-43620

@DrewKimball DrewKimball added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. branch-master Failures and bugs on the master branch. T-sql-queries SQL Queries Team A-sql-trigger Triggers and Trigger Functions backport-24.3.x Flags PRs that need to be backported to 24.3 labels Oct 19, 2024
@github-project-automation github-project-automation bot moved this to Triage in SQL Queries Oct 19, 2024
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Oct 24, 2024
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 cockroachdb#132979

Release note: None
@rytaft rytaft moved this from Triage to Active in SQL Queries Oct 29, 2024
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Nov 4, 2024
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 cockroachdb#132979

Release note: None
craig bot pushed a commit that referenced this issue Nov 6, 2024
133328: sql: fix trigger interaction with computed columns r=DrewKimball 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

134313: opt: make `max-stack` opt tester option more reliable r=mgartner a=mgartner

The `max-stack` opt tester option now runs the test command in a
separate goroutine. A fresh stack makes tests using this setting more
reliable.

It also decreases the `max-stack` of the original test that motivated
the `max-stack` option (see #132701) to 100KB, between 65KB in which the
test fails after the fix in #132701 and 135KB in which the test fails
before the fix.

Finally, the test has been disabled under `race` builds which increase
the size of stack frames and would cause this test to fail.

Epic: None

Release note: None


Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Nov 6, 2024
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 cockroachdb#132979

Release note: None
craig bot pushed a commit that referenced this issue Nov 6, 2024
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>
@craig craig bot closed this as completed in 43d6d62 Nov 6, 2024
@github-project-automation github-project-automation bot moved this from Active to Done in SQL Queries Nov 6, 2024
Copy link

blathers-crl bot commented Nov 6, 2024

Based on the specified backports for linked PR #133328, I applied the following new label(s) to this issue: branch-release-24.3. Please adjust the labels as needed to match the branches actually affected by this issue, including adding any known older branches.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added the branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 label Nov 6, 2024
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Nov 7, 2024
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 cockroachdb#132979

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-trigger Triggers and Trigger Functions backport-24.3.x Flags PRs that need to be backported to 24.3 branch-master Failures and bugs on the master branch. branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant