use hook_schema_alter() to declare the changes made to the database [#2044171]

It would be a really good idea to perform this module's database changes using hook_schema_alter() rather than directly changing the tables in hook_(un)install().

Comments

Comment #1

jenlampton

she / her

CreditAttribution: jenlampton commented 23 December 2016 at 21:57

Issue summary:	View changes
Status:	Active	» Postponed (maintainer needs more info)

It looks like hook_schema_alter() doesn't actually alter any tables. All it does is modify the schema variable representing tables. We'll setill need the un/install and update hooks to actually change the database structure. Additionally, I couldn't find any documentation on how to add fulltext indexes via hook_schema_alter().

If anyone knows how to do that, patches welcome :)

Comment #2

joachim CreditAttribution: joachim at Torchbox commented 24 December 2016 at 14:49

Title:	use hook_schema_alter() rather than direct database manipulation	» use hook_schema_alter() to declare the changes made to the database
Status:	Postponed (maintainer needs more info)	» Active

> It looks like hook_schema_alter() doesn't actually alter any tables.

Indeed, you're right -- I misread and misunderstood what it does.

The docs say:

When a module modifies the database structure of another module (by changing, adding or removing fields, keys or indexes), it should implement hook_schema_alter() to update the default $schema to take its changes into account.

So basically, this module is right to alter the tables directly, but it should also declare the changes it's made in a hook_schema_alter() implementation.