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

sql: support disabling constraints in-place #19444

Open
dt opened this issue Oct 23, 2017 · 19 comments
Open

sql: support disabling constraints in-place #19444

dt opened this issue Oct 23, 2017 · 19 comments
Labels
A-schema-changes C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@dt
Copy link
Member

dt commented Oct 23, 2017

Currently we always check/enforce a constraint if it exists. In some cases an operator may wish to skip the enforcement of constraints, e.g. during bulk loading, and then re-enable it later and do a one-time validation pass.

We have currently have unvalidated constraints, and the ability to explicitly validate an unvalidated constraint, but even unvalidated constraints are enforced.

A disabled constraint would be unenforced, skipping the extra work (e.g. FK lookups) required to enforce it. Re-enabling a disabled constraint would first need to move it to unvalidated, wait for that version to propagate, then validate.

Jira issue: CRDB-5973

@dt dt added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) feature good first issue help wanted Help is requested / needed by the one who filed the issue to fix it. labels Oct 23, 2017
@dianasaur323 dianasaur323 added this to the 1.3 milestone Oct 23, 2017
@bdarnell
Copy link
Contributor

How is disabling a constraint in this way different from dropping it and re-adding it later?

@dt
Copy link
Member Author

dt commented Oct 23, 2017

in theory it is no different -- you could transcribe all the constraints from SHOW CREATE TABLE, drop them and then re-add them later with ALTERs. In practice though, it seems it is usually paired with the ALL keyword to provide an easier, single-statement on/off switch, with the added polish of not having to worry about messing up any of the constraint defs in the process since they never leave the DB.

@cuongdo
Copy link
Contributor

cuongdo commented Oct 26, 2017

@dt is SET CONSTRAINTS what you were thinking of?

@jordanlewis
Copy link
Member

SET CONSTRAINTS is more advanced I think - that's about setting the deferred status of constraints, which we don't support yet.

I believe @dt was referencing the ability to leave the constraint in the table definition, but temporarily globally disable its enforcement. Postgres provides a way to do this via the DISABLE TRIGGER ALL syntax which isn't a great fit for us probably since we don't support triggers. https://stackoverflow.com/questions/38112379/disable-postgresql-foreign-key-checks-for-migrations

@ghost
Copy link

ghost commented Oct 31, 2018

Is anyone working on this issues ?

@dt
Copy link
Member Author

dt commented Nov 15, 2018

@darshanmurthy not that I know of

@bithavoc
Copy link

bithavoc commented Apr 9, 2019

This would be great, I can imagine that while running tests a database cleaner will:

  • Disable constraints
  • Truncate tables
  • Re-enable constraints

@MartinNiederl
Copy link

I would like to use this package to test the database, but currently this is not possible due to this issue: https://github.com/go-testfixtures/testfixtures

@dt Are there any plans for implementing this functionality?

@damienhollis
Copy link
Contributor

We have exactly the same need as @bithavoc - without being able to disable foreign keys, it makes it different to cleanup after running certain tests.

Any plans to add this to cockroachdb in the near future?

@ruzaikr
Copy link

ruzaikr commented Dec 9, 2019

I'd like to take this issue? Pls assign it to me?

@daniel-crlabs
Copy link
Contributor

I was designing a tool to generate random records for databases provided to us by customers or via debug zip. The customers or debug zip would provide us the ddl for the tables. The script would then create random records for each table and allow us to troubleshoot problems with actual data, but randomized data. However, this for now can not work, since some tables do have foreign key constraints, so it is a somewhat harder to programmatically find out the foreign key relationships and which tables to insert records first.
I do see this github issue was closed in favor of this one. If this feature exists, it would make this much simpler. What's the status on creating something to disable constraints in-place? Many other databases technologies support this, so it wouldn't be something un-heard of.

Example in MySQL:

Disable it:

SET FOREIGN_KEY_CHECKS=0;

Enable it:

SET FOREIGN_KEY_CHECKS=1;

@daniel-crlabs
Copy link
Contributor

Adding additional context to this, in hopes of maybe helping this become a feature.
Assume a DBA has to load a new environment and it is given hundreds of ddl files for each database, and each of these ddl files containing 100's of tables.
It would be rather cumbersome to edit all of these ddl files manually to remove the foreign key constraints and create additional alter statements to then add the foreign keys.
It would be much simpler to load all of these ddl files if there was a way to disable foreign key checks and then re-enable them, so the process would be:

1 - disable foreign key checks
2 - load ddl files
3 - enable foreign key checks

@rafiss
Copy link
Collaborator

rafiss commented Sep 17, 2021

I stumbled across this issue after @keithdoggett looked into slowness in running tests for our ActiveRecord adapter: https://github.com/cockroachdb/activerecord-cockroachdb-adapter

It takes about 1.5 to 2 hours to run these tests against CockroachDB. The same tests take about 5 minutes against PostgreSQL.

The source of the slowness was tracked down to the fact that we modified the tests to drop and re-add all constraints during each test setup: https://github.com/cockroachdb/activerecord-cockroachdb-adapter/blob/5e20ea12c5074357047886e8106ca02c36090b2d/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb

@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Sep 17, 2021
@rafiss rafiss added T-sql-queries SQL Queries Team and removed T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Sep 17, 2021
@ajwerner
Copy link
Contributor

How come this is in the @cockroachdb/sql-experience board? In practice, I think the disabling would be a schema change and then re-enabling would be one too. That wouldn't help with the testing speed but it's the only implementation I can reasonably conceive of. I'm sure the testing speed is, on some level, being misattributed.

@rafiss
Copy link
Collaborator

rafiss commented Oct 12, 2021

@ajwerner This issue is not currently on the SQL Experience board, but my comment above explains why I initially was interested it. I put it on @cockroachdb/sql-queries since it seemed like the proposal further up was for a session variable that disables FK checks at execution time.

But I haven't thought through the solution. If it's more of a @cockroachdb/sql-schema thing, then sounds fine to me!

@rytaft
Copy link
Collaborator

rytaft commented Oct 12, 2021

Happy to send it over! It's possible we'll need to set some flags in the optimizer to disable foreign key checks, but I agree the bulk of the work will be in schema. I'm skeptical that a session variable is the right way to handle this -- I think we'll want this to be a descriptor change. Reassigning to @cockroachdb/sql-schema.

@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Oct 12, 2021
@rytaft rytaft removed their assignment Oct 12, 2021
@ajwerner
Copy link
Contributor

Yeah, I don't think we can safely use a session variable. The query planner needs to know whether or not it can rely on constraints and we need to be careful about how we transition between disabled and enabled.

@Freeaqingme
Copy link

I would very much appreciate it if this could be implemented before the 'cockroach dump' command is definitely phased out. It does exactly what I need (in a dev environment with smallish datasets), and having the feature that's described in this issue would allow one to rather easily replicate the functionality in a standalone tool without having to bother about the exact order in which rows/tables are inserted.

@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label Aug 25, 2022
@gnat
Copy link

gnat commented Aug 31, 2022

This would be a nice performance boost for Django projects as well because Django ORM forces FK usage in models- the option to disable constraints on prod or during bulk queries would be nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-schema-changes C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests