-
Notifications
You must be signed in to change notification settings - Fork 621
Disallow dropping column trigger #4057
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
base: main
Are you sure you want to change the base?
Disallow dropping column trigger #4057
Conversation
|
Hi, EDIT: refer:https://github.com/sqlite/sqlite/blob/9b10698cc7dbe8fdbc29a36cdf9ac2b1f348cdc5/src/alter.c#L2194C1-L2263C65 - we rewrite and reload or run test statements in sqlite. |
| } { | ||
| "CREATE TABLE s (a, b, FOREIGN KEY (b) REFERENCES t(a, c))" | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could u please move this test to triggers or anywhere else , - ideally to a file present in all.test
we don't test this file against sqlite for compatibility
refer: https://discord.com/channels/1258658826257961020/1265743446828781669/1441391189109637182
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this to alter_table.test
| CREATE TRIGGER trg BEFORE DELETE ON t BEGIN UPDATE t1 SET m = 'x' WHERE m = 'y'; END; | ||
| ALTER TABLE t1 DROP COLUMN m; | ||
| } { | ||
| "Runtime error: error in trigger trg after drop column: no such column: m" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are expecting a error, u can just use do_execsql_test_in_memory_any_error
Isn't the way that we do better, i can just exclude the case where the trigger is on the same table |
Fixes #4029