Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Updated: Comment #32
Problem/Motivation
The support for foreign keys in the field schema is nearly completely broken, and only works by chance:
- The processing done in
_field_sql_storage_schema()
uses an undefined variable name - The foreign keys are not updated when the field is updated by
field_update_field()
We have some tests, but they pass by chance.
Proposed resolution
- Fix the brokenness
- Extend the tests
Remaining tasks
A patch has been created and applied to 8.x, and a patch exists and is RTBC for 7.x. Last remaining item is to commit to 7.x.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#29 | field-foreign_keys-1416506-29-D7.patch | 6.92 KB | yched |
#29 | interdiff.txt | 807 bytes | yched |
#27 | field-foreign_keys-1416506-27-D7.patch | 6.1 KB | yched |
#27 | interdiff.txt | 1.34 KB | yched |
#25 | field-foreign_keys-1416506-25-D7.patch | 6.05 KB | yched |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedTest only patch.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedTest+fix.
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedDrupal 7 version of the same.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedFix for the missing key in
field_update_field()
.Comment #8
andros CreditAttribution: andros commented#6: 1416506-field-foreign-keys.patch queued for re-testing.
Comment #10
David_Rothstein CreditAttribution: David_Rothstein commented#1969048: hook_field_schema() cannot vary based on field settings is a related issue (and possible duplicate).
Comment #11
Dave ReidBumping to major. This affects several contribs and blocks a regression from moving entityreference into core: #1847582: Create a foreign key to the target entity type base type in Entity-reference.
Comment #12
swentel CreditAttribution: swentel commentedWill need rerolling now CMI conversion is in. Tagging as well.
Comment #13
swentel CreditAttribution: swentel commentedWill reroll later this evening
Comment #14
swentel CreditAttribution: swentel commentedOk, this is trickier after the config conversion, will look later unless someone beats me to it.
Comment #15
swentel CreditAttribution: swentel commented- edit -
Sorry for no interdiff, had todo drupal_get_schema('table', TRUE) as we're in a unit test.
Comment #17
swentel CreditAttribution: swentel commentedFixing tests. I guess Damien or Yched can sign this off now - if ok of course :)
Comment #19
swentel CreditAttribution: swentel commentedDuh.
Comment #20
yched CreditAttribution: yched commentedLooks good.
Just minor adjustments:
- Fixed stale code comments
- Removed t()s around test messages
Comment #22
yched CreditAttribution: yched commentedTrailing whitespace.
+ needless double quotes.
Comment #23
webchickCommitted and pushed to 8.x. Thanks!
Comment #24
Dave ReidComment #25
yched CreditAttribution: yched commentedBackport.
Comment #27
yched CreditAttribution: yched commentedHm, is that better ?
Comment #29
yched CreditAttribution: yched commentedRight, pre-Field/CMI this requires a fix in field_update_field(), and @Damz's original patches accounted for that.
Scratch #27, interdiff is against #25.
Comment #30
malberts CreditAttribution: malberts commentedThis is not a proper review, but I patched 7.22 with the 4 non-test-related lines in that patch (#29). It appears to have fixed the problem in #1340748: Add CTools relationship.
I'm not sure if it's worse to hack 7.22 in my case or if I should rather run 7.x-dev + patch as I need this pretty much right now.
Comment #31
andypostawesome!
Comment #32
mixxmac CreditAttribution: mixxmac commentedThis worked for me in combination with #44 from #1340748: Add CTools relationship. After applying these patches, I was able to add a Taxonomy from Node relationship in Panels. That wasn't showing as an option before for that field, because it was using an entity reference instead of a term reference.
This helps a lot. Thanks for working on it!
Comment #33
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/9988e47
Comment #34.0
(not verified) CreditAttribution: commentedUpdated issue summary.