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

Don't warn about foreign keys causing table scan when creating table in same transaction #289

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

andrewsmith
Copy link
Contributor

I started to put this together to address an extension of #220: the case where the table is created and a foreign key constraint is added in the same transaction. We use a tool that generates SQL from a language-specific mapper and it separates these into two steps:

-- instead of 
CREATE TABLE foo (
  user_id BIGINT REFERENCES users (id),
  ...
);

-- or
CREATE TABLE foo (
  user_id BIGINT,
  ...
  CONSTRAINT user_id_fk FOREIGN KEY user_id REFERENCES users (id)
);

-- it generates
CREATE TABLE foo (
  user_id BIGINT,
  ...
);
ALTER TABLE foo ADD CONSTRAINT user_id_fk FOREIGN KEY user_id REFERENCES users (id);

In this case, there should be no table scan or it should be near instantaneous. I put together this change to adapt this rule to recognize this, but there are some shortcomings.

If someone were to have INSERT or COPY statements between the table creation and the foreign key addition, then the table scan becomes a real risk again. It also seems fairly intractable to determine this, as INSERT/COPY statements could be hidden in a stored procedure or function that is executed between the CREATE TABLE and ALTER TABLE commands.

Anyhow, I wanted to put this up for consideration and see if you had any thoughts on that problem. I imagine the INSERT/COPY between creation and foreign key addition would be pretty unlikely, but would cause Squawk to produce a false-negative, which is unsettling.

These are used heavily elsewhere and are a little better at verifying
the rule output than the hand-written assertions were.
These cover a few cases that should not cause this rule to warn because
the table is being created in the same transaction as the foreign key
constraint.
This addresses sbdchd#220, which claims that although an exclusive lock will
be obtained on the referenced table, there are no rows in the
referencing table so the table scan under the exclusive lock should be
short or non-existent.
@netlify
Copy link

netlify bot commented Mar 3, 2023

👷 Deploy request for squawkhq pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit ba7d66e

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.

1 participant