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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Status: Active » Postponed (maintainer needs more info)

The documentation says:

IMPORTANT NOTE: To maintain database portability, you have to explicitly recreate all indices and primary keys that are using the changed field.

That means that you have to drop all affected keys and indexes with db_drop_{primary_key,unique_key,index}() before calling db_change_field(). To recreate the keys and indices, pass the key definitions as the optional $keys_new argument directly to db_change_field().

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?

Clearbrook’s picture

I'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:

Only those keys that are directly utilizing the changed field as part of the key structure need to be dropped and added back as desired, but those keys must be dropped.

after the place where you say:

IMPORTANT NOTE: To maintain database portability, you have to explicitly recreate all indices and primary keys that are using the changed field.

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.

jhodgdon’s picture

Title: This does not work if the primary key is type serial in MySQL » db_change_field() docs are not clear about which indexes need to be deleted/rebuilt
Status: Postponed (maintainer needs more info) » Active

Hey... 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?

Clearbrook’s picture

I 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.

jhodgdon’s picture

See 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.

Clearbrook’s picture

I 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...

Clearbrook’s picture

I am not sure how compound keys might play into this. So even with this, there is still room for people to get confused....

jhodgdon’s picture

API 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.

Clearbrook’s picture

Thanks, 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.

;'{)

jhodgdon’s picture

Easy to figure out where docs are: https://drupal.org/node/144223
Making patches: http://drupal.org/novice

Clearbrook’s picture

Thanks! I am not sure how soon I can get on this, but I will make it a priority...

Live long and prosper!

;'{)

apaderno’s picture

Title: db_change_field() docs are not clear about which indexes need to be deleted/rebuilt » Make more explicit in Schema::changeField() which indexes need to be rebuilt
Version: 7.x-dev » 9.4.x-dev
Category: Bug report » Task
Issue summary: View changes

The change must first be done in the most recent release and eventually back-ported to Drupal 7.

apaderno’s picture

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Amber Himes Matz’s picture

The 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.

apaderno’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
pooja saraah’s picture

Rerolled patch against Drupal 10.1.x
Attached patch

apaderno’s picture

Issue tags: -Needs reroll

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.