Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too. Nancy E. Wichmann, PMP Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I don't think there is a problem with it at all. I've done it before. That's what its there for. To build on Brian's comments though you should use it in conjunction with hook_install/uninstall using db_add/drop_field in those functions and letting the schema API know about it in hook_schema_alter ----- Adam A. Gregory Drupal Developer & Consultant Web: AdamAGregory.com Twitter: twitter.com/adamgregory Phone: 910.808.1717 Cell: 919.306.6138 On Mon, Jul 12, 2010 at 12:30 PM, Brian Vuyk <brian@brianvuyk.com> wrote:
I don't see any problem with it, although I would add / remove the columns with hook_install() / hook_uninstall() rather than hook_schema_alter().
Brian
On 10-07-12 12:19 PM, nan wich wrote:
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
*Nancy E. Wichmann, PMP*
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
Le lundi 12 juillet 2010 à 09:19 -0700, nan wich a écrit :
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I would say, create a new table instead of populating core one. Each time you'll update your statistics, it will update the core database row, which should have not been updated. It will break most of the DBMS query cache, you will drastically lower your site performances if you have a lot of data. Ever worst, some DBMS do important physical row moves on each update (like PostgreSQL), this is no good, if you don't have to update a row, don't update it. You will may also experience some weird behaviors, like other modules dropping rows and recreating them identically, then loosing your statistics if they do not use drupal_write_record(). Pierre.
Pierre, if the data was that important, I would add it to another table and maybe even normalize it. This is stuff like the name and company of the commenter - both about as important as the email address that is already there. This is just not serious stuff in this case. Nancy E. Wichmann, PMP Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr. ________________________________ From: Pierre Rineau <pierre.rineau@makina-corpus.com> To: development@drupal.org Sent: Mon, July 12, 2010 12:33:35 PM Subject: Re: [development] (no subject) Le lundi 12 juillet 2010 à 09:19 -0700, nan wich a écrit :
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too. Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I would say, create a new table instead of populating core one. Each time you'll update your statistics, it will update the core database row, which should have not been updated. It will break most of the DBMS query cache, you will drastically lower your site performances if you have a lot of data. Ever worst, some DBMS do important physical row moves on each update (like PostgreSQL), this is no good, if you don't have to update a row, don't update it. You will may also experience some weird behaviors, like other modules dropping rows and recreating them identically, then loosing your statistics if they do not use drupal_write_record(). Pierre.
I always look at the use case when deciding if I should add to a core table or go with a separate table all together. Basically I base it on any queries that are going to run. In your situation, I would add to the core table if you will be setting sort and query criteria to the core columns and your new ones. If you aren't going to query on those columns, or sort on them, then a second table is probably better and just join it. So: SELECT n.*, c.* FROM node n INNER JOIN customer c ON c.nid=n.nid WHERE n.status=1 AND n.promote=1 ORDER BY n.created DESC That would be fine since all the selection and sort stuff is done on the node table, but: SELECT n.*, c.* FROM node n INNER JOIN customer c ON c.nid=n.nid WHERE n.status=1 AND n.promote=1 AND c.customer="acme" ORDER BY n.created DESC That would be better adding the columns to a core table, since you are going to get into some nastier queries that can easily go to tablesorts and temporary tables. Just my $.02 Jamie Holly http://www.intoxination.net http://www.hollyit.net On 7/12/2010 12:46 PM, nan wich wrote:
Pierre, if the data was that important, I would add it to another table and maybe even normalize it. This is stuff like the name and company of the commenter - both about as important as the email address that is already there. This is just not serious stuff in this case.
/*Nancy E. Wichmann, PMP*/
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
------------------------------------------------------------------------ *From:* Pierre Rineau <pierre.rineau@makina-corpus.com> *To:* development@drupal.org *Sent:* Mon, July 12, 2010 12:33:35 PM *Subject:* Re: [development] (no subject)
Le lundi 12 juillet 2010 à 09:19 -0700, nan wich a écrit :
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I would say, create a new table instead of populating core one.
Each time you'll update your statistics, it will update the core database row, which should have not been updated. It will break most of the DBMS query cache, you will drastically lower your site performances if you have a lot of data. Ever worst, some DBMS do important physical row moves on each update (like PostgreSQL), this is no good, if you don't have to update a row, don't update it.
You will may also experience some weird behaviors, like other modules dropping rows and recreating them identically, then loosing your statistics if they do not use drupal_write_record().
Pierre.
Pierre Rineau wrote:
Le lundi 12 juillet 2010 à 09:19 -0700, nan wich a écrit :
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I would say, create a new table instead of populating core one.
I would create a new table as well. The problem with modifying the core table is you never know if there will be a column added to it that matches a name you've chosen and if you add prefixes/suffixes to help namespace the column it becomes annoyingly long. -- Earnie -- http://progw.com -- http://www.for-my-kids.com
Le lundi 12 juillet 2010 à 13:27 -0400, Earnie Boyd a écrit :
Pierre Rineau wrote:
Le lundi 12 juillet 2010 à 09:19 -0700, nan wich a écrit :
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I would say, create a new table instead of populating core one.
I would create a new table as well. The problem with modifying the core table is you never know if there will be a column added to it that matches a name you've chosen and if you add prefixes/suffixes to help namespace the column it becomes annoyingly long.
More than that, if all modules would have done that, we'd probably all suffer from name conflicts from one module to another. Too much database alteration is not that good, keeping stuff separated on many tables can be such a mess for an external eye, but it keeps a clean encapsulation and can be a lot faster. Don't be afraid of JOIN, SQL has been done for this, just don't forget to put the right indexes and all will play nicely. The less you update other's tables (and the more small are the table you update) the less you will suffer from ineffective query cache and tables growing insanely (once I dealt a 8GB watchdog table because of a missing cron run, and most DBMS will allocate the size for it, but won't be able to unallocate it, which can be really problematic for the underlying FS). Pierre.
Le lundi 12 juillet 2010 à 09:19 -0700, nan wich a écrit :
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I misread what you said (cf. my first mail). But whatever you do, doing JOIN does not affect that much performances, as long as you have indexes on both table join column. Pierre.
my .02 is that such altering is quite reasonable for custom programming but not so kosher for contrib modules On Mon, Jul 12, 2010 at 12:19 PM, nan wich <nan_wich@bellsouth.net> wrote:
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
+1 for moshe's .02. Contrib should not, IMO, try to modify that extensively. Reference point is CCK, which gained simplicity in its inclusion in Core, but did not assume it. Kosher Law notwithstanding. On Mon, Jul 12, 2010 at 3:04 PM, Moshe Weitzman <weitzman@tejasa.com> wrote:
my .02 is that such altering is quite reasonable for custom programming but not so kosher for contrib modules
On Mon, Jul 12, 2010 at 12:19 PM, nan wich <nan_wich@bellsouth.net> wrote:
Is there an official stance on using hook_schema_alter to add columns to core tables? For example, we collect additional data on anonymous comments and want to actually save that data. Rather than creating another table (and subsequent JOINs), I'd just as soon stuff that data into the comments table, where it belongs. Should we ever disable comments (unlikely), we probably wouldn't mind losing that data too.
Nancy E. Wichmann, PMP
Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr.
I generally warn people away from modifying another module's tables (core being a collection of modules). The potential for collision, confusion, and stuff randomly disappearing on you is too great. If you have a one-off where you know what your situation is, then you can sometimes get away with it. A contrib module on d.o that starts messing with the user table, for instance, is a code smell that I want to stay far away from it. JOINs, especially on indexed integer fields (like nid, uid, etc.) are really really fast. SQL is designed to make them fast. Don't fear the join. --Larry Garfield On 7/12/10 2:53 PM, Brian Fending wrote:
+1 for moshe's .02. Contrib should not, IMO, try to modify that extensively. Reference point is CCK, which gained simplicity in its inclusion in Core, but did not assume it. Kosher Law notwithstanding.
On Mon, Jul 12, 2010 at 3:04 PM, Moshe Weitzman <weitzman@tejasa.com <mailto:weitzman@tejasa.com>> wrote:
my .02 is that such altering is quite reasonable for custom programming but not so kosher for contrib modules
On Mon, Jul 12, 2010 at 12:19 PM, nan wich <nan_wich@bellsouth.net <mailto:nan_wich@bellsouth.net>> wrote: > Is there an official stance on using hook_schema_alter to add columns to > core tables? For example, we collect additional data on anonymous comments > and want to actually save that data. Rather than creating another table (and > subsequent JOINs), I'd just as soon stuff that data into the comments table, > where it belongs. Should we ever disable comments (unlikely), we probably > wouldn't mind losing that data too. > > > Nancy E. Wichmann, PMP > > Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, > Jr.
First, in case I didn't say it, this is for an internal customer module, period. I would not be thinking of this for a contributed module, much as I would like to in some cases (especially pre-D7 taxonomy). I don't fear JOINs. In this case, it just seems like the best place to put this data is with the other stuff to wchich it belongs. In this case, that is with the comment row. As it turns out, it doesn't make much difference because the comment module doesn't use drupal_write_record, so I would need a hook_comment to add those fields any way. What a pain! Nancy E. Wichmann, PMP Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, Jr. ________________________________ From: "larry@garfieldtech.com" <larry@garfieldtech.com> To: development@drupal.org Sent: Mon, July 12, 2010 4:47:59 PM Subject: Re: [development] (no subject) I generally warn people away from modifying another module's tables (core being a collection of modules). The potential for collision, confusion, and stuff randomly disappearing on you is too great. If you have a one-off where you know what your situation is, then you can sometimes get away with it. A contrib module on d.o that starts messing with the user table, for instance, is a code smell that I want to stay far away from it. JOINs, especially on indexed integer fields (like nid, uid, etc.) are really really fast. SQL is designed to make them fast. Don't fear the join. --Larry Garfield On 7/12/10 2:53 PM, Brian Fending wrote:
+1 for moshe's .02. Contrib should not, IMO, try to modify that extensively. Reference point is CCK, which gained simplicity in its inclusion in Core, but did not assume it. Kosher Law notwithstanding.
On Mon, Jul 12, 2010 at 3:04 PM, Moshe Weitzman <weitzman@tejasa.com <mailto:weitzman@tejasa.com>> wrote:
my .02 is that such altering is quite reasonable for custom programming but not so kosher for contrib modules
On Mon, Jul 12, 2010 at 12:19 PM, nan wich <nan_wich@bellsouth.net <mailto:nan_wich@bellsouth.net>> wrote: > Is there an official stance on using hook_schema_alter to add columns to > core tables? For example, we collect additional data on anonymous comments > and want to actually save that data. Rather than creating another table (and > subsequent JOINs), I'd just as soon stuff that data into the comments table, > where it belongs. Should we ever disable comments (unlikely), we probably > wouldn't mind losing that data too. > > > Nancy E. Wichmann, PMP > > Injustice anywhere is a threat to justice everywhere. -- Dr. Martin L. King, > Jr.
participants (9)
-
Adam Gregory -
Brian Fending -
Brian Vuyk -
Earnie Boyd -
Jamie Holly -
larry@garfieldtech.com -
Moshe Weitzman -
nan wich -
Pierre Rineau