Coming from #633332: hook API documentation files have no information about where hooks are to go: It seems clear that implementations of hook_schema_alter() SHOULD live in .install files. But do they have to? Is what hook_schema_alter() does undone on de-installation? If not, should we add a note saying so?
Comments
Comment #1
jhodgdonI don't think that hook_schema_alter() belongs in a .install file, and as things are now I doubt it would work.
The thing is, this hook alters *other* modules' hook_schema implementations. My understanding of the module .install file is that it is for functions related to installing/enabling/disabling *this* module. Not everyone agrees -- see #1308610: move the location of hook_modules_uninstalled() to install files -- maybe this issue should be combined with that one, as it's kind of related in my mind at leat?
Comment #2
sven.lauer commentedAh, you are right. Apparently, I was not thinking clearly (somehow I was thinking of schema_alter()ing as an on-install task. But the hook is called whenever the schema information is rebuilt, so it likely needs to be in the .module file.
Leaving this open because:
1. Should we document that hook_schema_alter() needs to / should go into the .module file? I know this is the silent default, but given hook_schema_alter()s close connection to hook_schema() (which MUST be in the .install file), an additional note might not hurt?
2. The precise use and effect of hook_schema_alter() could be made clearer. The current version says:
Implicit in this is that just implementing hook_schema_alter() will NOT change the database (at least for already-installed modules). Maybe we should make this explicit?
In general, it is a little unclear (to me) how one would use hook_schema_alter() in conjunction with hook_(un)install(): I.e. how is the following scenario to be handled:
Module A alter()s module B's schema and, in A_install(), it makes the corresponding changes to the database.
Now suppose that A is installed before B. A's schema_alter() will be available and called when schema information is collected. However, since A's hook_install() was called before B even was there, the database will go unaltered. This would mean that the schema information and the database are out of sync, right?
Perhaps the intention is that a module should only schema_alter() parts of the schema of modules it depends on (i.e. in the above example, A should depend on B, preventing the scenario from happening). If so, this should be documented.
Comment #3
jhodgdonRE #2 - I see no problem with idea #1.
Idea #2... I don't know the answer to your question, but presumably if module B is altering module A's schema, module B would have module A as a dependency, so you would never be able to have module B enabled/installed without module A also being enabled, right? Also, just consider that API doc is not where to write a tutorial on how to write a module. The place for that is the online drupal.org/documentation, or the Examples for Developers project. All of that said, if you can think of some short snappy updates to the doc that would clarify it, I'm all for it. :)
Comment #4
sven.lauer commentedWell, I was not suggest to provide a full explanation of this use case (or a tutorial for using this function), but rather I was tryting to illustrate how I don't quite understand what the hook does / is supposed to do after reading the current doc. Minimally if this
is actually an assumption that drupal makes, it should be documented in the API doc (and this is not obvious---think about module A that providing optional integration with B if it is present, then you would not have a dependency, but A might still do something to B's schema (though perhaps it should not).
Comment #5
dave reidI'm not quite sure I understand why this hook needs to move from .install profiles. I can see clearly that drupal_get_complete_schema() from drupal_get_schema() loads all module's .install files. It's always been my assumption that if you are altering another schema, you also need to create your fields/indexes manually in your module's hook_install() (with checks like db_field_exists() to ensure you're not attempting to add it if it exists) as well as remove them in your hook_uninstall(). Maybe we just need to document this?
Comment #8
jp.stacey commentedAs I think the thread is in agreement that (a) hook_schema_alter() has to be in .module, (b) .module is the default and (c) changing this behaviour would be a code issue, not a documentation issue; and as this has been untouched for five years: I'm going to close it.