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

Removed exponential time complexity for foreign key analysis #1272

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

Hydrocharged
Copy link
Contributor

This fixes #1268

Foreign key analysis created acyclical trees that were traversed during query execution to emulate cascade operations. This meant that cyclical foreign keys were converted to an acyclical tree. Normally this isn't possible as cyclical trees are infinitely traversable, but MySQL has a depth limit of 15, which allowed us to materialize an acyclic tree with a maximum height of 15 nodes. This, however, lead to trees with an exponential number of nodes: roughly (number_of_fks)¹⁵ × 1.5 nodes in the tree. With just 3 foreign keys, we'd get a tree with roughly 22 million nodes, which would take forever to process.

This PR completely changes the analysis step to now generate cyclical trees. In addition, depth checks are now properly implemented (during query execution rather than during analysis), being represented by a returned error once the depth limit has been reached. Interestingly, MySQL is supposed to process up to 15 operations (returning an error on the 16th), but cyclical foreign keys will error on the 15th operation. I think this is a bug in MySQL, but nonetheless the behavior has been duplicated here.

I also updated the timestamp_test.go file to grab an unused port. This prevents test failures due to requesting an already-in-use port. Not related to this PR in particular, but it was annoying to deal with so I fixed it.

Copy link
Contributor

@andy-wm-arthur andy-wm-arthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. I tested against #1268 and the query executes instantly. Thanks Daylon!

@Hydrocharged Hydrocharged merged commit aa94dc1 into main Sep 20, 2022
@Hydrocharged Hydrocharged deleted the daylon/fk-infinite-loop branch September 20, 2022 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analyzing self-referential foreign keys triggering an infinite loop
2 participants