Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Schema::changeField()
shows the following note.
IMPORTANT NOTE: To maintain database portability, you have to explicitly recreate all indices and primary keys that are using the changed field.
I would make it more explicit by adding a sentence, changing that note to the following one.
IMPORTANT NOTE: To maintain database portability, you have to explicitly recreate all indices and primary keys that are using the changed field. Only those indexes that directly use the changed field need to be dropped and re-added.
Comment | File | Size | Author |
---|---|---|---|
#18 | reroll_diff_13-18.txt | 766 bytes | pooja saraah |
#18 | 2056059-18.patch | 721 bytes | pooja saraah |
| |||
#13 | drupal_make_more_explicit_which_indexes_should_be_rebuilt_2056059_13.patch | 721 bytes | apaderno |
Comments
Comment #1
jhodgdonThe documentation says:
This seems to say that you only have to drop indices that use the field you are changing
In your code, you are not attempting to change the field that is the primary key, so you should not drop the primary key... I guess I don't understand what the problem is with the documentation. What do you think is unclear, and what do you think it should say?
Comment #2
Clearbrook CreditAttribution: Clearbrook commentedI'll agree that one way to interpret that is as you are saying here. However, it is not immediately clear to the point of being unambiguous. The documentation needs to be concise. That does not always mean short and punctual. It does pay to reiterate points phrased differently, in order to insure that there is no alternate way to interpret what is being said.
I am not saying that the documentation is incorrect. I am saying that it is not so explicitly clear as to make alternate interpretations impossible. IF I am told I need to do XYZ in order to maintain compatibility, and I am dutifully trying to do so, I will tend to interpret XYZ in as broad a sense as I read so as to *not* risk failing to do XYZ fully. That is what is happening here.
I now understand that the way you wanted me to interpret this is the way I hoped it should be interpreted. That is good for me. However, this point may arise for others. I think that if this key were not a serial key, I would have dropped all the keys needlessly, not gotten an error message, and happily gone on my way, leaving behind code that dropped and rebuilt keys it did not need to. That could very well have happened with others.
I think adding the sentence:
after the place where you say:
I may be a dummy for not making the explicit connection. Others may be as dense as I am. You cannot assume that I will explicitly make the connection. Our experiences and levels of expertise differ. Explain it to a certified DBA as you have, and you are likely (but not guaranteed) to get it interpreted correctly. Explain it to a newbie, and they are going to scratch their head more likely than not, and stumble on with questions in their mind as to whether or not they are doing it correctly.
My point is: Consider the lowest possible common denominator of your potential audience. Don't talk down to them, but give them the information they need to be clear on the concept. We are not all DBA's, and we should do everything possible to encourage people to rely upon the documentation in their attempts to contribute, no matter how trivial their contributions may seem to be. I actually have some understanding and even formal training (albeit minimal) in relational databases. This tripped me up, at least for a bit. If not for my minimal background, I would not have even thought to question the meaning of what was said in the documentation, and very well may have given up in frustration.
You have knowledge many of us do not. You help us all by passing it on, and I thank you for that. But, if it is not passed on to the broadest possible audience, we do ourselves a disservice. Whatever you decide to do, I will not lose sleep over it. But I think it is worth the effort to improve this for those to follow behind me who may not be as fortunate to have figured it out eventually.
Comment #3
jhodgdonHey... I was not at all trying to imply you were in any way dense, and I'm sorry if anything I wrote implied that! I was just trying to figure out what was wrong and/or unclear in the documentation.
I am not at all against adding to the documentation to make it clearer, and would welcome a patch. Perhaps you would like to make the patch?
Comment #4
Clearbrook CreditAttribution: Clearbrook commentedI was not thinking you were implying such. My understanding was that you did not understand why it was not clear to me. I do think my understanding is less than yours. That was my point. If the implication was that I was not up to snuff to understand, I am more than understanding of that point. I would even agree with such an assessment. Referring to myself as dense was merely a way of making my proposition more palatable, as it were...
As for a patch, I have never done a patch for documentation. How is that done? I am more than willing to do so, but am unsure as to how it is done.
Comment #5
jhodgdonSee http://drupal.org/novice for patch instructions and instructions on how to get help making patches.
But as a first step, how about listing out here what you think needs to be added to the documentation, or what needs to be clarified? I am still not sure.
Comment #6
Clearbrook CreditAttribution: Clearbrook commentedI have used git to make patches to code, but not to API documentation. I am not sure how to git the API documentation -- unlike the modules, I don't see a link like the 'Version control' that they have. I looked at the link you just gave me and am still scratching my head on that one...
Soo...
-IMPORTANT NOTE: To maintain database portability, you have to explicitly recreate all indices and primary keys that are using the changed field.
+IMPORTANT NOTE: To maintain database portability, you have to explicitly recreate all indices and primary keys that are using the changed field. Only those keys that are directly utilizing the changed field as part of their key structure need to be dropped and added back as desired, but those keys must be dropped.
Kind of a pseudo patch. Just to add one clarifying sentence.
And maybe you could point me to how to git the API documentation to make an actual patch. I'll probably say 'Doh!' again when you explain it to me, but I am just not figuring that one out. Sorry...
Comment #7
Clearbrook CreditAttribution: Clearbrook commentedI am not sure how compound keys might play into this. So even with this, there is still room for people to get confused....
Comment #8
jhodgdonAPI documentation is in the /** */ documentation headers immediately before functions, right there in the code, if you would like to make a patch. The api.drupal.org site is entirely generated from documentation in the code.
Comment #9
Clearbrook CreditAttribution: Clearbrook commentedThanks, I will look into that. If I find it, I'll make the patch. It may take me a while, but I should be able to grep it and find it.
;'{)
Comment #10
jhodgdonEasy to figure out where docs are: https://drupal.org/node/144223
Making patches: http://drupal.org/novice
Comment #11
Clearbrook CreditAttribution: Clearbrook commentedThanks! I am not sure how soon I can get on this, but I will make it a priority...
Live long and prosper!
;'{)
Comment #12
apadernoThe change must first be done in the most recent release and eventually back-ported to Drupal 7.
Comment #13
apadernoComment #16
Amber Himes MatzThe patch in #13 does contain the original suggestion. The comment in #7 is never (in 10 years) clarified with a patch or actual suggestion, so I'm ignoring that. I'm going to submit it for re-testing in current versions to see if it still applies and then I'm OK with it moving forward.
Comment #17
apadernoComment #18
pooja saraah CreditAttribution: pooja saraah at Srijan | A Material+ Company for Drupal India Association commentedRerolled patch against Drupal 10.1.x
Attached patch
Comment #19
apaderno