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: udf already being dropped in local-legacy-schema-changer #96368

Closed
rharding6373 opened this issue Feb 1, 2023 · 0 comments · Fixed by #97631
Closed

sql: udf already being dropped in local-legacy-schema-changer #96368

rharding6373 opened this issue Feb 1, 2023 · 0 comments · Fixed by #97631
Assignees
Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rharding6373
Copy link
Collaborator

rharding6373 commented Feb 1, 2023

Describe the problem

During a drop column cascade, there's an error that one of the UDFs is already being dropped.

To Reproduce

In a logic test using the local-legacy-schema-changer configuration, the following statements result in the error. The ordering and/or quantity of UDFs seems important to reproducing the error.

statement ok
CREATE TABLE t_twocol (a INT, b INT);

statement ok
CREATE FUNCTION f_unqualified_twocol() RETURNS t_twocol AS
$$
  SELECT t_twocol.a, t_twocol.b FROM t_twocol;
$$ LANGUAGE SQL;

statement ok
CREATE FUNCTION f_allcolsel_alias() RETURNS t_twocol AS
$$
  SELECT t1.a, t1.b FROM t_twocol AS t1, t_twocol AS t2 WHERE t1.a = t2.a;
$$ LANGUAGE SQL;

statement ok
CREATE FUNCTION f_tuplestar() RETURNS t_twocol AS
$$
  SELECT t_twocol.a, t_twocol.b FROM t_twocol;
$$ LANGUAGE SQL;

# This statement should be ok but fails
statement ok
ALTER TABLE t_twocol DROP COLUMN b CASCADE;

Error in output:

expected success, but found
(XXUUU) function "f_tuplestar" is already being dropped
drop_function.go:167: in dropFunctionImpl()

Expected behavior
I expect the DROP to succeed and all the functions to be dropped as part of the cascade.

Jira issue: CRDB-24090

@rharding6373 rharding6373 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 1, 2023
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Feb 1, 2023
@chengxiong-ruan chengxiong-ruan added the branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 label Feb 1, 2023
@chengxiong-ruan chengxiong-ruan self-assigned this Feb 1, 2023
chengxiong-ruan added a commit to chengxiong-ruan/cockroach that referenced this issue Feb 24, 2023
Fixes: cockroachdb#97546
Fixes: cockroachdb#96368

We are reusing the underlying slots of the `DependedOnBy`
slice when removing items from it. This is problematic because
we normally have to iterate the same slice to figure out what
to drop. Iterating on a slice which is rewritten can be problematic
unless we always know how to smartly move the cursor backward.
This commit remove such kind of hazard by collecting descriptor
IDs to drop before actually dropping everything.

Releaste note (sql change): This commit fixes two bugs prevent
users from doing `DROP COLUMN CASCADE` and `DROP INDEX CASCADE`
when dependecies on the Table from Views and Functions get complex.
They all happened When a table is referenced multiple times in
more than one Views and Functions.

Release note: None.
chengxiong-ruan added a commit to chengxiong-ruan/cockroach that referenced this issue Feb 27, 2023
Fixes: cockroachdb#97546
Fixes: cockroachdb#96368

We are reusing the underlying slots of the `DependedOnBy`
slice when removing items from it. This is problematic because
we normally have to iterate the same slice to figure out what
to drop. Iterating on a slice which is rewritten can be problematic
unless we always know how to smartly move the cursor backward.
This commit remove such kind of hazard by collecting descriptor
IDs to drop before actually dropping everything.

Releaste note (sql change): This commit fixes two bugs prevent
users from doing `DROP COLUMN CASCADE` and `DROP INDEX CASCADE`
when dependecies on the Table from Views and Functions get complex.
They all happened When a table is referenced multiple times in
more than one Views and Functions.

Release note: None.
chengxiong-ruan added a commit to chengxiong-ruan/cockroach that referenced this issue Feb 28, 2023
Fixes: cockroachdb#97546
Fixes: cockroachdb#96368

We are reusing the underlying slots of the `DependedOnBy`
slice when removing items from it. This is problematic because
we normally have to iterate the same slice to figure out what
to drop. Iterating on a slice which is rewritten can be problematic
unless we always know how to smartly move the cursor backward.
This commit remove such kind of hazard by collecting descriptor
IDs to drop before actually dropping everything.

Releaste note (sql change): This commit fixes two bugs prevent
users from doing `DROP COLUMN CASCADE` and `DROP INDEX CASCADE`
when dependecies on the Table from Views and Functions get complex.
They all happened When a table is referenced multiple times in
more than one Views and Functions.

Release note: None.
craig bot pushed a commit that referenced this issue Feb 28, 2023
97631: sql: create new slice when removing backreference items r=chengxiong-ruan a=chengxiong-ruan

Fixes: #97546
Fixes: #96368

Previously we reused the underlying slots of the `DependedOnBy` slice when removing items from it. This is problematic because we normally have to iterate the same slice to figure out what to drop. Iterating on a slice which is rewritten can be problematic unless we always know how to smartly move the cursor backward. This commit changes it to just create a new slice to avoid the hazzard.

Release note: None.

Co-authored-by: Chengxiong Ruan <[email protected]>
@craig craig bot closed this as completed in 2e05878 Feb 28, 2023
chengxiong-ruan added a commit to chengxiong-ruan/cockroach that referenced this issue Feb 28, 2023
Fixes: cockroachdb#97546
Fixes: cockroachdb#96368

We are reusing the underlying slots of the `DependedOnBy`
slice when removing items from it. This is problematic because
we normally have to iterate the same slice to figure out what
to drop. Iterating on a slice which is rewritten can be problematic
unless we always know how to smartly move the cursor backward.
This commit remove such kind of hazard by collecting descriptor
IDs to drop before actually dropping everything.

Releaste note (sql change): This commit fixes two bugs prevent
users from doing `DROP COLUMN CASCADE` and `DROP INDEX CASCADE`
when dependecies on the Table from Views and Functions get complex.
They all happened When a table is referenced multiple times in
more than one Views and Functions.

Release note: None.
@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. GA-blocker T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants