-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Foreign key on update action with non literal values #14278
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Harshit Gangal <[email protected]>
0d6c8e7
to
fc08a7d
Compare
Signed-off-by: Manan Gupta <[email protected]>
…mns and literals for update queries Signed-off-by: Manan Gupta <[email protected]>
… queries Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
…lumns being set to non-literal values Signed-off-by: Manan Gupta <[email protected]>
fc08a7d
to
9b191a5
Compare
Signed-off-by: Manan Gupta <[email protected]>
…iteral expressions Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
… updates Signed-off-by: Manan Gupta <[email protected]>
9bf3419
to
a1b04e4
Compare
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
… equality check Signed-off-by: Manan Gupta <[email protected]>
…ing the query to fail Signed-off-by: Manan Gupta <[email protected]>
…gether in a struct Signed-off-by: Manan Gupta <[email protected]>
@harshit-gangal Could you review it again please 🥺 ? |
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.
LGTM
…st use TryExecute instead Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
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.
Approving on Harshit's behalf.
Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
Description
This PR adds support for foreign key on update action when the referenced value is non-literal.
eg:
update tbl set fk_col = cola + colb where id = 100
.Planning is augmented to support this use case. We also had to change the
FkCascade
engine to add this functionality. For the case of non-literal updates, we have to propagate the cascades by going over each row one by one since the value that each row is updated to could be different for each one.As part of this PR, the fuzzer has also been augmented to produce queries with complex expressions to make sure everything works as intended.
We still have one limitation after this PR wherein we cannot support column updates for foreign keys such that it depends on some other column that is also being updated. For example, consider the query
update u_tbl3 set cola = id, colb = 5 * (cola + (1 - (cola))) where id = 2
where colb is part of a foreign key constraint. In this case, we cannot safely run this query in Vitess while propagating all the cascade operations. Therefore, we have chosen to fail queries like these.We cannot support updating a foreign key column that is using a column that is also being updated for 2 reasons—
Post these changes, we are able to support non-literal updates to foreign key columns (barring the limitation listed above). An example plan for such a plan looks like -
Related Issue(s)
Checklist