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

[YCQL] Prevent DROP TYPE for the used YCQL type #17107

Closed
1 task done
Anagha-Damodaran opened this issue May 1, 2023 · 1 comment
Closed
1 task done

[YCQL] Prevent DROP TYPE for the used YCQL type #17107

Anagha-Damodaran opened this issue May 1, 2023 · 1 comment
Assignees
Labels
area/ycql Yugabyte CQL (YCQL) kind/bug This issue is a bug priority/high High Priority

Comments

@Anagha-Damodaran
Copy link

Anagha-Damodaran commented May 1, 2023

Jira Link: DB-6394

Description

The current functionality allows for dropping a User-Defined Type (UDT) even if it is still being referenced. This can lead to unexpected behavior and errors in applications that rely on the UDT. To prevent this, it would be beneficial to add constraints to the DROP TYPE command to prevent dropping a UDT that is still being used.

Implementing such constraints can ensure that dropping a UDT only happens when it is safe to do so, and it can provide better control over the system's data types. This can help prevent errors and ensure that the system remains stable and reliable.

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@Anagha-Damodaran Anagha-Damodaran added area/ycql Yugabyte CQL (YCQL) status/awaiting-triage Issue awaiting triage labels May 1, 2023
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 1, 2023
@m-iancu
Copy link
Contributor

m-iancu commented May 2, 2023

Adding a repro here. It is working correctly for top-level UDTs, but it looks like nested/frozen UDTs are not properly checked.

Set up a UDT

ycqlsh> create keyspace sample;
ycqlsh> use sample;
ycqlsh:sample> create type tp(a int, b text);

Top-level UDT test

ycqlsh:sample> create table tbl(k int primary key, v1 tp, v2 int);
ycqlsh:sample> insert into tbl(k, v1, v2) values (1, { a : 2, b : 'abc'}, 3);
ycqlsh:sample> drop type tp;
InvalidRequest: Error from server: code=2200 [Invalid query] message="Invalid Request. Cannot delete type 'sample.tp'. It is 
used in column v1 of table tbl (master error 29)
drop type tp;
          ^^
 (ql error -316)"

^ correctly disallows dropping the UDT.

Frozen UDT test

ycqlsh:sample> drop table tbl;
ycqlsh:sample> create table tbl2(k int primary key, v1 frozen<tp>, v2 int);
ycqlsh:sample> insert into tbl2(k, v1, v2) values (1, { a : 2, b : 'abc'}, 3);
ycqlsh:sample> drop type tp;
ycqlsh:sample> select * from tbl2;

 k | v1                | v2
---+-------------------+----
 1 | tp(a=2, b=u'abc') |  3

^this should fail but succeeds.
Note that the table is still usable because the type information is stored expanded in the table schema (not just a reference to the type). So this is only an issue when we try to do backups.

@yugabyte-ci yugabyte-ci added priority/high High Priority and removed priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage labels May 9, 2023
OlegLoginov added a commit that referenced this issue May 11, 2023
Summary:
The current functionality allows dropping a User-Defined Type (UDT) even if it is still being referenced.
Only if the type is used directly as a column type - it cannot be deleted.
A reference from a collection or frozen is ignored now.
This can lead to unexpected behavior and errors in applications that rely on the UDT.

`CatalogManager::DeleteUDType` is updated to prevent dropping used UDT which is still being used.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, yguan, stiwary

Reviewed By: stiwary

Subscribers: yql, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D25229
OlegLoginov added a commit that referenced this issue May 19, 2023
…ions

Summary:
In the original commit:  aca0f6d / D25229
the test `TestUserDefinedTypes#testNestedUDTsWithCollections` uses information
that the table `tableName3` is the first in the list of tables.
That's correct for `2.18` and for the Master.
But in branches <= `2.16` the table order is not predictable due to random table UUIDs.

The test is updated to expect any order of the tables.
E.g. for the first `DROP TYPE udt4` the type is used by `tableName1` and `tableName2`
and `tableName3`. So the error message can mention any from these 3 tables.

The test is updated in the Master to have it synced with other branches where the
table order is random.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, yguan, stiwary

Reviewed By: stiwary

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D25464
OlegLoginov added a commit that referenced this issue May 19, 2023
Summary:
Original commit: aca0f6d / D25229
GH link: aca0f6d

The current functionality allows dropping a User-Defined Type (UDT) even if it is still being referenced.
Only if the type is used directly as a column type - it cannot be deleted.
A reference from a collection or frozen is ignored now.
This can lead to unexpected behavior and errors in applications that rely on the UDT.

`CatalogManager::DeleteUDType` is updated to prevent dropping used UDT which is still being used.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, stiwary, yguan

Reviewed By: stiwary

Subscribers: bogdan, yql

Differential Revision: https://phorge.dev.yugabyte.com/D25300
OlegLoginov added a commit that referenced this issue May 19, 2023
Summary:
Original commit: aca0f6d / D25229
GH link: aca0f6d

The current functionality allows dropping a User-Defined Type (UDT) even if it is still being referenced.
Only if the type is used directly as a column type - it cannot be deleted.
A reference from a collection or frozen is ignored now.
This can lead to unexpected behavior and errors in applications that rely on the UDT.

`CatalogManager::DeleteUDType` is updated to prevent dropping used UDT which is still being used.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, stiwary, yguan

Reviewed By: stiwary

Subscribers: bogdan, yql

Differential Revision: https://phorge.dev.yugabyte.com/D25299
OlegLoginov added a commit that referenced this issue May 19, 2023
Summary:
Original commit: aca0f6d / D25229
GH link: aca0f6d

The current functionality allows dropping a User-Defined Type (UDT) even if it is still being referenced.
Only if the type is used directly as a column type - it cannot be deleted.
A reference from a collection or frozen is ignored now.
This can lead to unexpected behavior and errors in applications that rely on the UDT.

`CatalogManager::DeleteUDType` is updated to prevent dropping used UDT which is still being used.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, stiwary, yguan

Reviewed By: stiwary

Subscribers: bogdan, yql

Differential Revision: https://phorge.dev.yugabyte.com/D25304
OlegLoginov added a commit that referenced this issue May 19, 2023
Summary:
Original commit: aca0f6d / D25229
GH link: aca0f6d

The current functionality allows dropping a User-Defined Type (UDT) even if it is still being referenced.
Only if the type is used directly as a column type - it cannot be deleted.
A reference from a collection or frozen is ignored now.
This can lead to unexpected behavior and errors in applications that rely on the UDT.

`CatalogManager::DeleteUDType` is updated to prevent dropping used UDT which is still being used.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, stiwary, yguan

Reviewed By: stiwary

Subscribers: bogdan, yql

Differential Revision: https://phorge.dev.yugabyte.com/D25313
OlegLoginov added a commit that referenced this issue May 19, 2023
Summary:
Original commit: aca0f6d / D25229
GH link: aca0f6d

The current functionality allows dropping a User-Defined Type (UDT) even if it is still being referenced.
Only if the type is used directly as a column type - it cannot be deleted.
A reference from a collection or frozen is ignored now.
This can lead to unexpected behavior and errors in applications that rely on the UDT.

`CatalogManager::DeleteUDType` is updated to prevent dropping used UDT which is still being used.

Test Plan: ybd --java-test org.yb.cql.TestUserDefinedTypes#testNestedUDTsWithCollections

Reviewers: mihnea, stiwary, yguan

Reviewed By: stiwary

Subscribers: bogdan, yql

Differential Revision: https://phorge.dev.yugabyte.com/D25317
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ycql Yugabyte CQL (YCQL) kind/bug This issue is a bug priority/high High Priority
Projects
None yet
Development

No branches or pull requests

4 participants