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: investigate circular dependency restrictions between udfs and relations #108223

Closed
rharding6373 opened this issue Aug 4, 2023 · 5 comments · Fixed by #118583
Closed

sql: investigate circular dependency restrictions between udfs and relations #108223

rharding6373 opened this issue Aug 4, 2023 · 5 comments · Fixed by #118583
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. good first issue T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rharding6373
Copy link
Collaborator

rharding6373 commented Aug 4, 2023

The following example in logic tests fails in CRDB due to circular dependency check:

> CREATE TABLE t_circle(a INT PRIMARY KEY, b INT);
> CREATE FUNCTION f_circle() RETURNS INT LANGUAGE SQL AS $$ SELECT a FROM t_circle $$;
> ALTER TABLE t_circle ADD CONSTRAINT ckb CHECK (b + f_circle() > 1);

ERROR: error executing StatementPhase stage 1 of 1 with 3 MutationType ops: *scop.AddTableConstraintBackReferencesInFunctions: &{{{}} 104 2 [105]}: cannot add dependency from descriptor 104 to function f_circle (105) because there will be a dependency cycle

However, this is inconsistent with postgres, which allows this behavior:

CREATE TABLE t_circle(a INT PRIMARY KEY, b INT);
CREATE FUNCTION f_circle() RETURNS INT LANGUAGE SQL BEGIN ATOMIC; SELECT a FROM t_circle; END;
ALTER TABLE t_circle ADD CONSTRAINT ckb CHECK (b + f_circle() > 1);
\d t_circle;
--              Table "public.t_circle"
--  Column |  Type   | Collation | Nullable | Default 
-- --------+---------+-----------+----------+---------
--  a      | integer |           | not null |  
--  b      | integer |           |          | 
-- Indexes:
--     "t_circle_pkey" PRIMARY KEY, btree (a)
-- Check constraints:
--     "ckb" CHECK ((b + f_circle()) > 1)
INSERT INTO t_circle VALUES (0,0);
INSERT INTO t_circle VALUES (1,0);
-- ERROR:  23514: new row for relation "t_circle" violates check constraint "ckb"

This came up as part of auditing CRDB's use of pg error codes against postgres #107369. If this discrepancy is expected, please add an error code to the test and a note.

Jira issue: CRDB-30368

@rharding6373 rharding6373 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 4, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Aug 4, 2023
@rafiss
Copy link
Collaborator

rafiss commented Aug 9, 2023

Although it's a divergence, we think it's better to show the error earlier, rather than waiting until the INSERT occurs. But we should improve the error message and add a PG Code.

@rafiss rafiss added good first issue E-quick-win Likely to be a quick win for someone experienced. E-easy Easy issue to tackle, requires little or no CockroachDB experience labels Aug 9, 2023
@chettriyuvraj
Copy link

New here! Can I pick this up?

@zyf123123
Copy link
Contributor

Hello @chettriyuvraj , If you are not working on this one im gonna to take up this and try :)

@rharding6373
Copy link
Collaborator Author

I don't think there is an open PR on this one. You're welcome to pick this up. Thank you for contributing!

@zyf123123
Copy link
Contributor

I have already commit a PR for this issue, please review my code. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. good first issue T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants