Consider alternatives to modifying core tables

This is a dupe of the code style issue.

The .install file adds a field to the core users table. That's not accepted procedure for adding configuration.

If you can point me to some links that back up your claims, and also show why this isn't accepted procedure, I will consider changing the code accordingly.

In the meantime, I'll explain why I did it this way. Any user data in an external table is not loaded into the $user object during the bootstrap process. This means that in order to load the data relevant to this module (which is one value from one column), a call to user_load() is required. However, user_load() calls user_module_invoke(), which will go through all modules and call hook_user() if they have an $op value of 'load', resulting in a database call for each module that has this value. This creates a fair bit of unnecessary overhead, seeing as only one value is needed from one table. And since this value would either need to be called on every page load during hook_init(), it means that this extra overhead would happen on every single page for every user.

Conversely, by setting up the module the way I did - adding the column to the users table, the value is loaded during the bootstrap process and requires no extra database calls that aren't already part of the bootstrap process.

So I think you can see why I did it this way.

Finally, if adding colums to pre-existing tables is wrong, why did they create the function hook_schema_alter().

To your credit, I was unable to find a link to an article (that I hadn't written) that explicitly states modding core schema from contrib is verboten. However, I performed a quick straw poll in #drupal and all of the developers that responded agreed that this is not the kind of behavior one would expect from a contrib module and that it is ill-advised.

The problem with modifying core schema is one of uncertainty. You have absolutely no way to account for how other contributed modules (or core code) is going to interact with the table you've modified.

For example, if you add a column to the users table and someone else's module is doing a quick and dirty INSERT INTO {users} VALUES(...) you get "ERROR 1136 (21S01): Column count doesn't match value count at row 1" or a similar error.

As anecdotal evidence of my point I submit the issues surrounding the Real Name module. To date the module maintainer has been filing issues all over the place trying to get other module maintainers to change how they display usernames. Similar issues have been filed with Facebook Connect. There's a core issue floating around now that proposes adding a "real name" field to the users table. The reason it hasn't been unilaterally implemented by contrib developers is because these kinds of mods are considered akin to hacking core.

Ultimately this is your module and (as I'm sure you are aware) you are free to do with it as you please, however I encourage you to look into alternate implementation strategies that don't involve tampering with core tables. This will greatly increase the chances of your module gaining support by the community.

In parting I'd also like to direct you to deekayen's profile page, specifically the part that indicates he has nine years of experience contributing to the project. He's got more lines of code submitted to the Drupal project than most of the developers at Lullabot. You might want to listen to the man.

Thank you for the reply.

Honestly, while I can see what you are saying, I can't think of a case whereby the changes I have made would cause problems. I have not changed anything that exists in the table, rather I have only added another column to the table. Unless another module were to maybe implement a similar column name (which is unlikely seeing as the column name is 'force password change'), having this extra data in the table should not affect other modules one way or another.

Now, if you or someone else can provide a real example of what can go wrong with this module based on the changes I have made, then I will definitely consider making the changes. But as it stands now, I don't see that the fairly significant additional overhead that I would have to add to each page load in order to prevent some unnamed potential problem makes it worthwhile.

As for Deekayen's experience - I'm glad to hear he has contributed so much to Drupal. Drupal is an excellent piece of software that has been made possible by people contributing to it - which is why I also try to contribute both with modules and in the forum. However, I am not going to change the module purely based on someone's reputation, that just isn't a logical approach to the situation. However changing the module based on actual problems that could arise is something I am open to if someone comes up with some.

#2123505: PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'base.force_password_change' may be either a duplicate of this issue, or an example of an actual problem.