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: test ON UPDATE interactions #69058

Closed
pawalt opened this issue Aug 17, 2021 · 8 comments · Fixed by #75846
Closed

sql: test ON UPDATE interactions #69058

pawalt opened this issue Aug 17, 2021 · 8 comments · Fixed by #75846
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@pawalt
Copy link
Contributor

pawalt commented Aug 17, 2021

Upon completion of #69057, we will need to test ON UPDATE interactions with other types of constraints. Of interest are:

  • Interaction with foreign key updates
  • Interaction with DEFAULT expressions
  • Interaction with backfills

Epic CRDB-9148

@pawalt pawalt added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 17, 2021
@rafiss rafiss added this to Triage in SQL Sessions - Deprecated via automation Sep 21, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Sep 21, 2021
@vy-ton
Copy link
Contributor

vy-ton commented Sep 21, 2021

@rafiss Do we need to do this for 21.2?

@otan
Copy link
Contributor

otan commented Sep 21, 2021

In particular:

  • Interaction with foreign key updates
    Ensure we can't create FKs with an ON UPDATE on the same column. Thinks that's done.
    When another table does an update on a FK that e.g. sets the value to NULL, do we want ON UPDATE to trigger? I'm guessing no?

  • Interaction with DEFAULT expressions
    ? checking having both populated works ?
    this already seems tested, but idk what peyton had in mind

  • Interaction with backfills
    Backfills over the row (e.g. ADD COLUMN) do not trigger an ON UPDATE change.

@rafiss
Copy link
Collaborator

rafiss commented Sep 21, 2021

thanks @otan !

@vy-ton for 21.2 would you be able to include the above in your acceptance testing? we will also keep this issue open and work on adding these tests during this milestone

@rafiss rafiss moved this from Triage to September 2021 in SQL Sessions - Deprecated Sep 21, 2021
@vy-ton
Copy link
Contributor

vy-ton commented Sep 22, 2021

I can check these scenarios

Ensure we can't create FKs with an ON UPDATE on the same column.
Is this a FK referencing the ON UPDATE column?

@otan
Copy link
Contributor

otan commented Sep 22, 2021

Ensure we can't create FKs with an ON UPDATE on the same column.
Is this a FK referencing the ON UPDATE column?

You can't have id INT ON UPDATE 2 REFERENCES(other-table.int) ON UPDATE SET NULL

@vy-ton
Copy link
Contributor

vy-ton commented Sep 27, 2021

  • Interaction with DEFAULT expressions
CREATE TABLE t1 (
  i int,
  ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
  d DATE DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
);
insert into t1(i) values (1), (2), (3);
update t1 set i = 10 where i = 1;
update t1 set (i, ts, d) = (select i, ts, d from t1 where i = 2) where i = 2;
  • Interaction with backfills: Backfills over the row (e.g. ADD COLUMN) do not trigger an ON UPDATE change.
vy@free-tier9.gcp-asia-southeast1.crdb.io:26257/defaultdb> CREATE TABLE inventories (
    product_id        INT,
    warehouse_id      INT,
    quantity_on_hand  INT DEFAULT 100 ON UPDATE 50,
    PRIMARY KEY (product_id, warehouse_id)
  );
INSERT INTO inventories (product_id, warehouse_id, quantity_on_hand) VALUES (1, 1, 1);
vy@free-tier9.gcp-asia-southeast1.crdb.io:26257/defaultdb> insert into inventories (product_id, warehouse_id) values (2, 2);
vy@free-tier9.gcp-asia-southeast1.crdb.io:26257/defaultdb> select * from inventories;
  product_id | warehouse_id | quantity_on_hand
-------------+--------------+-------------------
           1 |            1 |                1
           2 |            2 |              100
vy@free-tier9.gcp-asia-southeast1.crdb.io:26257/defaultdb> ALTER TABLE inventories ADD COLUMN new_col INT DEFAULT 13;
vy@free-tier9.gcp-asia-southeast1.crdb.io:26257/defaultdb> select * from inventories;
  product_id | warehouse_id | quantity_on_hand | new_col
-------------+--------------+------------------+----------
           1 |            1 |                1 |      13
           2 |            2 |              100 |      13
         
  • Interaction with foreign key updates
vy@free-tier9.gcp-asia-southeast1.crdb.io:26257/defaultdb> CREATE TABLE orders_2 (
    id INT PRIMARY KEY,
    customer_id INT ON UPDATE 10 REFERENCES customers_2(id) ON UPDATE CASCADE ON DELETE CASCADE
  );
ERROR: cannot specify both ON UPDATE expression and a foreign key ON UPDATE action for column with ID 2
SQLSTATE: 42P16

@rafiss
Copy link
Collaborator

rafiss commented Oct 15, 2021

We found one buggy interaction where ALTER TYPE (experimental) deletes the ON UPDATE clause: #71571

@rafiss rafiss moved this from September 2021 to October 2021 in SQL Sessions - Deprecated Nov 11, 2021
@rafiss rafiss self-assigned this Dec 6, 2021
@rafiss rafiss moved this from October 2021 to December 2021 in SQL Sessions - Deprecated Dec 6, 2021
@rafiss rafiss assigned ZhouXing19 and unassigned rafiss Jan 18, 2022
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Feb 2, 2022
craig bot pushed a commit that referenced this issue Feb 3, 2022
75779: sql: add the TTL automatic column & related variables r=postamar a=otan

See individual commits for details.

75842: sql: migration to remove orphaned primary index comments r=fqazi a=fqazi

Fixes: #73409

Previously, on older Cockroach versions it was possible
to drop indexes and not have comments for them cleaned up.
This was inadequate because entries would be left in
system.comments table for indexes that were dropped.
This patch, will introduce a new migration to clean up
index comments for any indexes that no longer exist.

Release note: None

75846: sql: test ON UPDATE's interaction with backfills over row r=otan a=ZhouXing19

Fixes #69058

Release Note: none

75859: migration/migrations: rework publicSchemaMigrationTest to not use con… r=ajwerner a=ajwerner

…stants

Needed for dynamic system table ID work. I think it makes the test cleaner.

Release note: None

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com>
Co-authored-by: Jane Xing <zhouxing@uchicago.edu>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
@craig craig bot closed this as completed in 3ffb164 Feb 3, 2022
@craig craig bot closed this as completed in #75846 Feb 3, 2022
SQL Sessions - Deprecated automation moved this from January 2021 to Done Feb 3, 2022
kvoli pushed a commit to kvoli/cockroach that referenced this issue Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants