Well, this is less than minor but... the schema description is wrong for the {cache_field} table, since it says:
Generic cache table for caching things not separated out into their own tables. Contributed modules may also use this to store cached items.
This comes from the direct copy of the generic {cache} table at line 166 of field.install:
$schema['cache_field'] = drupal_get_schema_unprocessed('system', 'cache');
| Comment | File | Size | Author |
|---|---|---|---|
| #52 | wrong_schema-892344-39.patch | 452 bytes | tstoeckler |
| #6 | 892344-field_.install.cache_description-6.patch | 473 bytes | pcambra |
Comments
Comment #1
LaurentAjdnik commentedThe description itself might need to be rewritten:
Comment #2
LaurentAjdnik commentedComment #3
swentel commentedMoving to 8.x, care to re-roll ? :)
Comment #4
swentel commented#2: 892344.field_.install.cache_description.patch queued for re-testing.
Comment #6
pcambraHere's a reroll
Comment #7
ramlev commentedReviewed and working
Comment #8
catchCommitted/pushed to 8.x, moving to 7.x for backport. The 7.x backport should technically have an update function to correct the schema definition for existing sites.
Comment #9
tstoecklerI'm not a native speaker, hence, not resetting this directly, but I'm pretty sure "informations" is not proper English. In that case we should roll a short follow-up to fix that typo before backporting this.
Comment #10
dcam commented#2: 892344.field_.install.cache_description.patch queued for re-testing.
Comment #11
dcam commentedThe "informations" text has been removed from D8, so this no longer needs a follow-up.
#2 does need to be rerolled for D7 though. As mentioned in #8, D7 will also need an update function.
Comment #12
amitgoyal commentedPlease review update patch as per fixes in #8.
Comment #13
brahmjeet789 commentedIn drupal 7.x-28, I have not seen any cache field description in field.install file but in patch #12 it includes cache description field in that file.
Comment #14
dcam commentedI'm going to mark #12 as RTBC, but with reservations. I don't think we should be using SQL queries in update functions. Unfortunately, I don't see any means in the Database API to alter a table's comment. David_Rothstein will need to weigh in on it.
Otherwise, the patch works as promised. The field_cache table description is set to the new text on install and on update.
Comment #19
mgiffordBack to RTBC.
Comment #22
dcam commentedComment #25
David_Rothstein commentedComment #28
dcam commentedComment #31
dcam commentedComment #36
dcam commentedComment #38
David_Rothstein commentedThis at least fails for SQLite (I tested); have a feeling it will fail for PostgreSQL too. Also, the update function should be inside the "updates-7.x-extra" block, not outside...
I'm not really sure what to do about this if there isn't a way to add a table comment in a cross-database-compatible way. Is it so bad to fix this for new installs only, and skip the update function?
Comment #39
hussainwebThe database issue is non-functional. In my opinion, it doesn't seem too bad to just skip the database update. I have attached a patch, which is basically the same as the patch in #6.
Comment #40
tstoecklerI agree it's fairly pointless to worry about an incorrect schema description on existing D7 sites at this point.
Comment #41
dcam commentedAgreed. I thought about this while travelling today and decided I would rather see the change get in for new sites than let this issue sit in limbo forever because the update isn't supported by the database API. RTBC +1
Comment #44
mgiffordComment #47
dcam commentedComment #50
dcam commentedComment #52
tstoecklerThat's a random fail. Re-uploading to keep the context of the fail.
Comment #53
David_Rothstein commentedCommitted to 7.x - thanks!