sql: provide a way to temporarily disable foreign key checks on a session basis #23624
Comments
|
How does this relate to DEFERRABLE constraints (#9897)? |
|
Also curious if the proposed functionality in #19444 meets the same use case. |
|
Specifically, if we did add a session setting like this instead of the DISABLE ALL mutation on the tables, I think it should a) require ALTER on the tables it writes to and b) must mark all FKs it ignores as unvalidated, if they’re not already, since it may have invalidated them. |
|
Oops. Dupe of #19444. This came up in context of testing TPCC - it would be nice to be able to disable foreign key checks via a session setting to test performance with and without those checks. |
|
Oh, wait. No, this is specifically about a session setting that evilly skips validating foreign key constraints regardless of the state of the schema. https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_foreign_key_checks
|
|
@jordanlewis can you clarify what the use case is? It sounds a bit like "I want a tool to shoot myself in the foot" |
|
I don't think this is a good idea. The only use case I can think of is for bulk inserts to existing already partially filled tables. Mysql has this option because they don't have I covered this in https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20171201_cascade.md See the https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20171201_cascade.md#survey-of-other-databases for how they all deal with it. |
|
I do not think we should implement it exactly as mysql does -- just disabling the check code. I think, if you've chosen to not check FKs at all, we need to record on those FKs that we can no longer make promises about their on-disk data... until you run |
|
This issue arose from a desire to temporarily disable the FK checks in order to determine their performance impact. I could be satisfied with an easy way to do this via recompilation. @jordanlewis pointed out that MySQL has a setting for it. I have had this desire, I imagine users might as well. It seems reasonable to make it clear how absolutely dangerous this setting is. |
|
👍 it does indeed seem like a not uncommon desire to quantify the cost of checking constraints. To clarify my concern, I'm not opposed to disabling checks, but rather that on that re-enabling them the database should not make potentially false claims. |
|
@dt Agreed. Either we need to document that the foreign keys might be invalid, or provide a mechanism for re-validating them. |
|
Yep, we have that mechanism for re-validating -- |
|
Related: #23663 you may want to extend the mechanism to also disable CHECK validation |
|
Re-thinking this with a cool head, I realize that if any one session would enable the setting to disable FK checks, then FK constraints potentially become invalidated for every other session, not just the one. For me this 100% turns this feature into a cluster-wide setting. Also the action of changing the setting from "on" (no FK checks) back to "off" (FK checks enabled) should edit all the table descriptors to mark constraints as |
|
Also, should a client violate the FK constraints, this means the table stats would go out of whack, and the query optimizer would subsequently start to make very bad choices. |
|
(I'm overall 👎 on this feature due to the disorderly spill of invalidity) |
knz
added
C-enhancement
|
Anyone who further looks into this please carefully envision the interaction with a user who mistakenly sets this and starts (unintendedly) corrupting their database. How would you explain this user how to put their database back into a valid state? How do you suggest they identify which writes were violating their FK constraints? How do you suggest they go back in history to fix/remove these writes (made with the setting enabled) and no other (made with the setting disabled)? |
|
The UX story here in the "bad case" is so supremely horrendous that it makes me shudder. Perhaps we should requalify this issue to become a build flag instead. |
|
@knz fwiw, I think it also makes sense to consider #19444 as an alternative -- disabling the FKs themselves in the schema via a schema change command. That makes it readily apparent that they're disabled to all observers, unlike per-session state, and has clear semantics for what happens after: re-enabling them is done explicitly, via a followup schema change, and can include (perhaps with an option) re-validation of them. |
|
Yes #19444 sounds much saner to me. |
|
I think we should close this, and if we want the ability to disable constraints checking, pursue the approach in #19444. I still think #19444 is a safer, less foot-gun solution to the underlying desire to temporarily disable constraints for performance reasons, but given that constraints are making promises about the table data, recording that they're disabled on the table seems much safer to me than just in one client's in-memory session. |
|
agreed |
…or a cockroachdb fork, to support writes at least * sql_ascii hopefully unnecessary (not even completely sure it's important for postgresql). * there's no equivalent to pg_relation_size etc. * there's no way to disable FK constraints (cockroachdb/cockroach#23624, closed in favor of cockroachdb/cockroach#19444 which wouldn't suit) to support reads, we'd also need to write out pg_export_snapshot(). * cockroachdb would use AS OF SYSTEM TIME ..., but it's hard to choose the value. * we should use statement_timestamp() or the semi-undocumented cluster_logical_timestamp(), but https://forum.cockroachlabs.com/t/use-cockroachdb-cluster-timestamp-in-time-travel-queries/2093/6 says that if you use a value less than --max-offset ago, you may not read your own writes. * and I can't even see a way to get the max-offset value, in order to workaround by sleeping that long.
MySQL provides an option
SET FOREIGN_KEY_CHECKS=0;which temporarily disables all foreign key checks on a session basis. This would be useful to have in CockroachDB as well.The text was updated successfully, but these errors were encountered: