API page: https://api.drupal.org/api/drupal/includes%21database%21database.inc/fun...

Enter a descriptive title (above) relating to function db_change_field, then describe the problem you have found:

Trying to follow this documentation, I have an error that crops up. Message I get is as follows:

Failed: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1075 Incorrect table definition; there can be only one auto column and it must be defined as a key: ALTER TABLE {file_managed} DROP PRIMARY KEY; Array ( ) in db_drop_primary_key() (line 2901 of /home1/xxxxxxx/public_html/home7.22/includes/database/database.inc).

I am using MySQL 5.5.32-log, Drupal 7.22, PHP 5.3.27, Apache version 2.2.25 through Hostmonster.

This code seems to work:

// db_drop_primary_key('file_managed');
db_drop_unique_key('file_managed');
db_drop_index('file_managed');

db_change_field('file_managed', 'type', 'type',
array('description' => 'The type of this file.', 'type' => 'varchar', 'length' => 50, 'not null' => TRUE, 'default' => 'undefined'),
array(
// 'primary key' => array('fid'),
'unique key' => array('uri'),
'index' => array('uid', 'status', 'timestamp', 'type'
)));

But, notice that I had to comment out the dropping of the primary key. From the API discussion, I would believe that could make this code break on PostgreSQL. I can be reading this too broadly, and hope I am. The field 'type' *is* an index, and if I read it narrowly enough, you may be saying that *if* the field I am trying to alter is an index or key, I may need to use db_drop_{primary_key,unique_key,index}() on all of the keys that pertain *only* to that column before proceeding. However, the way it is written, that is ambiguous, at best. I will try just dropping the key on that column and hope that the narrower reading of what is in the documentation is correct. Dropping *only* the keys on that field is an alternate way to read the documentation.

It may be that in order to do this correctly, I must get the actual schema from the database, move the data to a temporary table, and then drop and rebuild the table over again. I am hoping that is not needed...

My only concern is that the code I write works with all database engines. I would suggest the following to be be amended to make it clear, if my assumption is correct:

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 linked to the changed field need to be dropped and added back as desired, but those keys must be dropped.

If my assumption is *not* correct, please let me know.

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!

;'{)