Our core table names are inconsistently named, reflecting the history of how Drupal developed. Some, e.g., 'node', are named in the singular for the object type they hold. Others, e.g., 'users', are named in the plural. Still others, notably 'term_data', have a totally custom name.

As we begin to take giant steps forward in introducing a schema system, http://drupal.org/node/136171, opening the way for a consistent Data API across all object types, a key need is to bring consistency. A one-to-one relationship between object type names and their primary tables will greatly facilitate operations like drupal_save($type, $item); where $type is e.g. 'node', with primary table 'node'.

All of which is to say, it's time to adopt consistent table-naming.

The attached patch simply renames existing tables in the singular. E.g., 'comments' becomes 'comment'. 'term_data' becomes 'term'. I've also included the joining tables mapping many-to-many relationships. We already had 'term_node'. Now 'blocks_roles', becomes 'block_role', etc.

Since table names are already enclosed in curly brackets, this change will be easy to make for module maintainers--a simple search and replace. (The only difficulty I guess will be if there are existing contrib tables with conflicting names. Update script will need to detect existing tables and rename them.)

Still to do: the update script.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

I'm supportive for this. (Although, I don't think it is a showstopper for the data API to progress. I'd prefer to see us work on the data API first, and then worry about table name cleanup but I won't be picky.)

nedjo’s picture

FileSize
150.04 KB

Refreshing the patch after the Schema API being applied.

See http://drupal.org/node/145684 for some ideas of what this patch would open up.

webchick’s picture

Oh, YES PLEASE!!! This has bugged me forever.

Adding to my "definitely test" queue. Thanks, Nedjo!!

webchick’s picture

Oh, I should point out too a +1 to singular table names. There was a huge thread about table naming conventions I read a long time ago, since I'm a total sucker for that kind of stuff. ;) The consensus was that singular better because:
a) it's more consistent (for example, should the bridge table between users and roles be user_roles or users_roles or..?)
b) it's easier for non-English people to grasp, as they don't have to remember weird pluralization rules in English (boxES vs. blockS).
c) it makes more sense in most queries; you want to select a name from a particular user, not a name from all of the users (generally speaking).

webchick’s picture

FileSize
176.77 KB

Ok re-rolled with a few fixes:

  • There were still references to {users_roles} in user.module.
  • The files table wasn't renamed to file in system.schema.
  • There were still references to {menu_links} and {comments}.
  • The node_comment_statistics table wasn't renamed to node_comment_statistic (not sure if we want that or not, but I figured I'd keep them consistent).
  • term_data wasn't renamed to term.

I went around and clicked on most things and seemed to go ok. Definitely could use another set of eyes, though.

Also, an update path is missing. I need to read up in the documentation to figure out how to write that.

webchick’s picture

And for those following along at home, here are the name changes:

blocks => block
blocks_roles => block_role
boxes => box
comments => comment
files => file
file_revisions => file_revision
filters => filters
filter_formats => filter_format
languages => language
menu_links => menu_link
node_comment_statistics => node_comment_statistic
node_revisions => node_revision
poll_choices => poll_choice
poll_votes => poll_vote
profile_fields => profile_field
profile_values => profile_value
sequences => sequence
sessions => session
term_data => term
users => user
users_roles => user_role
vocabulary_node_types => vocabulary_node_type

This will break approximately every snippet in the handbook. ;) However, it makes the data schema FAR more consistent and easy to grok for new developers, and provides a consistent means to handling the Data API, through clever introspection.

nedjo’s picture

Assigned: nedjo » Unassigned

Thanks Angie! That was just the careful attention and fixing up the patch needed.

For the update, I'm not exactly sure what we need to do. Here's a suggested approach:

* Change names of all existing tables. Do this update in system module, so that it's run before other modules' updates. We have a relevant snippet from content.install:


  foreach ($rename as $old_name => $new_name) {
    switch ($GLOBALS['db_type']) {
      case 'mysql':
      case 'mysqli':
        $ret[] = update_sql("RENAME TABLE {". $old_name ."} TO {". $new_name ."}");
        break;

      case 'pgsql':
        $ret[] = update_sql("ALTER TABLE {". $old_name ."} RENAME TO {". $new_name ."}");
        break;
    }
  }

* Use the new names for all post-5.x updates in all modules' install and update functions. For example, php_install() includes handling of existing filter formats. I included one update function in the table name changing, locale_update_6001().)

Does that sound right?

I also wondered above about how to handle the case that the new names are already taken by contrib modules' tables, and suggested we simply rename those existing tables. Does that sound right?

(I'm unassigning myself in the hope I can leave this in your capable hands, but do let me know if there's ore I can do :) )

webchick’s picture

no, I think the update paths are different now, post-schema patch. I'll try and find out. :)

We'll also need to update the sequences table so that {node_revisions}_vid, for example, becomes {node_revision}_vid.

The whole 'renaming takes a name already taken by contrib' is... interesting. I hadn't thought of that. but yeah, probably something like check first if the table exists. If so, output an error and don't attempt the rename until it's resolved?

David Strauss’s picture

As long as we don't go the Rails direction with a giant pluralization dictionary (and this patch doesn't), this sounds like a good idea.

nedjo’s picture

See this patch http://drupal.org/node/147285 to add db_rename_table(), which would be useful here.

drewish’s picture

i think this would be a good change. it was a bit frustrating initially trying to remember the table names. i just hope the files patch that renames file_revisions to upload sticks before this.

Chris Johnson’s picture

I'm thrilled to see this patch!

I actually rolled a patch to rename all the core tables about a month ago, but then when I started addressing the problem of doing the update from D5 to D6, I very quickly realized that was the hard part. :-)

Whatever renames the tables has to operate in such a way to not interfere with the many other updates that need to run, as well.

I'm rather of the mind that contrib modules which conflict with renamed core database tables need to be altered to avoid the collision.

Actually, I also rather like the idea of making the table naming convention a hard and fast rule for contrib, too. If you write a module and use a table name that's plural, your module doesn't get published. But I'm sure there will be little support for that, nor any easy way to automate it. At least it should be so documented in the coding standards. Consistency really reduces effort, errors and improves quality, which is why we have coding standards, right?

Nedjo++ !

ChrisKennedy’s picture

Status: Needs review » Needs work

Massive hunk failures.

webchick’s picture

Assigned: Unassigned » webchick
Status: Needs work » Postponed

Ah, crap. I totally meant to re-roll this but little point in it now.

Will make this my first priority for D7, aka, "The fantastically CRUDdy release." ;)

chx’s picture

Version: 6.x-dev » 7.x-dev
Category: feature » task
Priority: Normal » Critical
Status: Postponed » Needs work

postponed now is less needed, maybe when a patch needs to postponed within the same cycle. adjusted status.

catch’s picture

Bumping this, if only because I can never remember which ones are plural and which aren't, and that's annoying.

Chris Johnson’s picture

Still bugs the heck out of me. :-)

BioALIEN’s picture

If we fix pluralisation, we can add something in coder.module to check this too :)

webchick’s picture

Assigned: webchick » Unassigned

Me too. But obviously, I'm not working on this anymore. :)

jwuk’s picture

I saw #4 'b' and my gut reaction was 'Hurrah!'. One of the joys of Drupal is the diversity of the users and contributors. On reflection though, irregularity of English pluralisation is probably not one of the highest hurdles a non-native speaker suffers.

This issue crops up in many projects, not just Drupal. Imagining the inconsistency to be about singular vs plural is to oversimplify it.

A table such as 'users' contains rows where each row describes a user. A 'menu_router' table is (I guess) so named because its purpose is to be used in routing, not because each row is a menu router.

If you want consistency you have to understand English grammar well enough to understand the role collectives play (and Fowler's Modern English Usage has seven sections in his article on this!).

So you would rename 'users' not 'user', but some choice such as 'population' or 'flock'. I'm not for an instant suggesting you do this, you'd be throwing the baby out with the bathwater. I'm just trying to explain why the original choices are not quite as illogical as they might seem at first sight.

recidive’s picture

Those kind of patches tend to be giant, so I suggest splitting it up by modules, so:

system.module

actions
actions_aid
blocked_ips
files
menu_links
sessions

block.module

blocks
blocks_roles
boxes

comment.module

comments

filter.module

filter_formats
filters

node.module

node_revisions

poll.module

poll_choices
poll_votes

user.module

users
users_roles

taxonomy.module

vocabulary_node_types

recidive’s picture

I've posted a patch for renaming user module tables: #330983: Rename user module tables to singular.

recidive’s picture

I've forgot to enable all core modules before making the previous list. Those ones also need renaming:

blogapi.module

blogapi_files

locale.module

languages

profile.module

profile_fields
profile_values

search.module

search_node_links

trigger.module

trigger_assignments

I think splitting by module makes sense, since we only need one update function per module.

recidive’s picture

Ok, block.module's tables got renamed.

I've submitted a new patch for renaming node, filter and comment module tables.

The upgrade path for user and system tables will be more difficult to write, since those tables are used for access check in update.php. So I'll work on renaming those tables on the same patch.

More coming.

Damien Tournoud’s picture

What is exactly the use of this? Some sort of research for a "conceptual beauty of table naming"?

I'm really -1 for all this. We shouldn't waste effort on such a trivial matter.

recidive’s picture

@Damien Tournoud: Who is wasting efforts? Is it you?

dww’s picture

Priority: Critical » Normal

A) Our coding standards say:

Use plural or collective nouns for table names since they are sets and not scalar values.

If we were going to standardize, we should standardize on our own stated coding standard. :(

B) Since when is this "critical"? ;)

Dave Reid’s picture

Wish this patch had waited until it was RTBC before actually being committed so we could have continued discussion about it. I don't feel the singular table names are fitting and it's an unnecessary change. It'd be like changing all "admin/*" paths to "administration/*" since we wouldn't want to use abbreviations.

Edit: referring to #335086: Rename node, filter and comment modules tables to singular and #342294: Rename poll, profile and taxonomy modules tables to singular which were both committed without being RTBC first. The latter being committed accidentally and still has not yet been un-committed. Steps like these should be RTBC first, should they not?

Damien Tournoud’s picture

I feel that this is an unecessary change. This will break virtually every patch out there, while the benefit of it looks really minimal. The cost of inconsistency here is minimal, so I'll vote for inconsistency.

Dave Reid’s picture

I'd prefer if #335086: Rename node, filter and comment modules tables to singular was rolled back as well so we can continue this discussion. Decide what we want to do. Change the standards, then apply the standards once approved.

recidive’s picture

"The latter being committed accidentally and still has not yet been un-committed. Steps like these should be RTBC first, should they not?"

Should Dries mark issues RTBC?

Should we wait patches to be RTBC to review them/comment on issues.

I am sorry, but I have the best intentions when I submit my patches.

Dave Reid’s picture

I understand completely, even I had an issue that was committed a little prematurely. It's up to us issue reviewers to mark issues as RTBC. On bigger, more wide-reaching patches like this one is, it's preferred to get the official RTBC-ok from someone a little higher up and that has more weight on this site than the majority of us.

David Strauss’s picture

@dww Then I'd support changing our coding standards to singular names for tables. SQL uses table names in both singular and plural contexts.

An example of a singular and plural context: "SELECT users.id FROM users WHERE users.name IS NOT NULL". The users.name condition applies to individual user rows, not the entire table's set of values.

Drupal also uses singular nouns in path names, even when there's a potentially plural context, so we wouldn't be breaking overall convention by using singular nouns in table names.

An example: /node and /node/1234

nedjo’s picture

Before the first patches were applied, this change received strong support from several Drupal contributors, including two core committers, and no negative reviews.

It's true that, for the multiple and long term benefits of this patch already noted, there will be a lot of updates needed in the short term. But, compared to many other updates, these are simple and easily automated. In most cases, a quick search and replace of e.g. {users} will do the trick.

Thanks recidive for your work on this series of patches.

dww’s picture

For the record: I withdraw my objection to singular table names -- I really don't care much about this. Consistency is good, and if there are any technical reasons why singular is better than plural, that's fine with me. I probably shouldn't have even commented. ;)

All I ask is that once this is done someone will:

A) Edit the coding standards I referenced to reflect the new standard and mention it in the 6.x to 7.x upgrade docs.

B) It might also be worth a note to the devel list, just to inform Drupal developers about the change in the standard so if they want to rename their contrib tables to be compliant they could start now.

C) It'd also be nice to provide a coder and/or deadwood upgrade rule that automates renaming the core tables when people are porting to 7.x. If nothing else, an issue in those respective queues would be nice.

Thanks,
-Derek

Dave Reid’s picture

I'm also loosening up to this as well. My major issue is still that these changes were being committed either accidentally, or without being RTBC. For issues that make such a wide-sweeping change, I would think we'd want the official RTBC-ok first.

boombatower’s picture

After a bit of searching it seems the tables left are: (I would really like to see this happen)

actions
actions_aid

#375373: Make blocked_ips table singular
blocked_ips

blogapi_files

#329301: Rename {files} to {file} and add unique key to the filepath column
files

languages

menu_links

node_comment_statistics

search_node_links

#375367: Make session table singular
sessions

trigger_assignments

#330983: Rename user module tables to singular
users
users_roles

EDIT: Adding issues as they are completed.

boombatower’s picture

Assigned: Unassigned » boombatower
Status: Needs work » Postponed

Since I'm fine finishing all these off I'll assign it to myself. I'll let the first three get in before I do any others.

drewish’s picture

i would really like to combine the files table renaming with #329301: Rename {files} to {file} and add unique key to the filepath column. because to make that happen we need to just do a new file table with a primary key on the path and have each module migrate their own data.

boombatower’s picture

We need a consistent way to upgrade users and sessions as they cause WSOD during upgrade.

boombatower’s picture

"re. http://drupal.org/node/330983 we usually add special code that needs to be run before bootstrap into a function in update.php i think there is one already search for d7" -chx

boombatower’s picture

update_prepare_d7_bootstrap();
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
drupal_maintenance_theme();

update_prepare_d7_bootstrap() seems to be our man...looks like we can create an update hook or something there and should be good to go...I'll work on this.

boombatower’s picture

Implemented on user rename patch...awaiting commit before updating other patches.

recidive’s picture

@boombatower: I have one question on your implementation of update_prepare_d7_bootstrap_rename():

Will it give the chance for admins to backup their database? Or will it rename tables just by accessing update.php?

Will it show the first upgrade page telling admins to backup their databases?

boombatower’s picture

No it won't.

I guess that is one small hitch...although it is clearly stated in all the upgrade documentation. Not sure what other options we have...if we load anything more than database it craps. WSOD.

Josh Waihi’s picture

This is bad. 'user' is a reserved name in PostgreSQL, not to mention a standard. boombatower and I and a lengthy discussion about this on IRC, having 'user' for a table name is not ideal because of database conflicts. While agreed it shouldn't be used, the alternative naming convention is still in question.

boombatower’s picture

Postgres seems to be alone in not supporting table name delimiters (couldn't find sqlite).

The standards are bellow, although all three reserve "user".

http://dev.mysql.com/doc/refman/5.0/en/reserved-words.html
http://msdn.microsoft.com/en-us/library/aa238507.aspx
http://www.postgresql.org/docs/7.3/static/sql-keywords-appendix.html

Singular is most definitely the way to go (as you can see that the dbs user it for internal storage), but not sure how to work around postgres. We can work on an alternate name, but just a note the following two solutions work.

schema.tablename (public.user)

prefixtablename (duser)

or some default prefix if non specified.

Josh Waihi’s picture

Status: Postponed » Needs work

if the table 'user' stays, this will really make things hard for postgres, 'SELECT * FROM user' doesn't return drupal's user table but the database's table. again, the table would have to be referenced as schema.tablename (public.user)

Damien Tournoud’s picture

Priority: Normal » Critical

"user" is a reserved word, and is actually listed here: http://drupal.org/node/141051

There is no way we convert users to user. And this proves my points in #25 and #29: this renaming is a complete waste of everyone's time. This issue should never have been opened.

jwuk’s picture

I wish I'd not spent time in #20 standing on an academic soapbox to no real benefit. I'm 100% with Damien (#49), I'm joining him in saying the Emperor has no clothes. If these changes go ahead they'll soak up a whole lot of development effort, debugging effort, documentation effort, ... and it's sure to piss off at least some 3rd party module maintainers. With what real added value?

But before anyone's brave enough to throw this out there'll be another 50 comments here, because it's one of those "what color shall we paint the bicycle shed" questions that Everyone has a view on.

I can see it was raised with the best of intentions, but unless developers are really really short of things to do I can't see why this is a good use of their precious time.

If a need arises for a one-to-one relationship between object type names and their primary tables, couldn't that be achieved in a gentler, less revolutionary fashion by having two little functions, one for each direction? Internally these functions would have a table listing the exceptions to the rule of matching names.

boombatower’s picture

@Josh Waihi: It makes no sense that SELECT * FROM user would return the database's users as they are in a different database information_schema. Instead I would expect that to result in a syntax error.

@Damien Tournoud: Just because one database cannot handle a table name is by NO means proof that it is a bad concept. Instead we should consider the fact that both databases use the table name "user" for there table. I find your conclusion rather rash to put it frankly.

I think it really opens up a new issue of correctly and fully supporting Postgres. Contrib modules have no-one to tell them, "You can't use this table name because Postgres doesn't like it." So just as in previous version of Drupal the contrib will be able to "break support" for Postgres. If you are to fully support this you need a way to ensure that any table name in a module either: a) breaks on all database consistently, or b) works on all databases. Anything in between leaves room for error which if everyone is so adamant about supporting other databases we should take the time to do right, not quickly revert this change and stray from the main issue.

@jwuk: Consistent naming is useful for abstract CRUD operations as the title suggest. It would open up things like drupal_save($object); Where object has a type of 'user', or 'node'. Wouldn't make sense to have a type of 'users' as it only contains a single user. Not to mention this battle has been fought in many communities with the same result. It is like using all verbs or all nouns in menu item names...choose a standard and go with it.

I have noticed a number of issues to make things more consistent. For instance the VERY minor appending (dot) syntax. That got in in the name of consistency. Having consistent tables name is a good thing and as I pointed out above, it points out a bigger problem.

I propose we make all table names work on all database, rather then trying to store a SUM() list of reserved words. Since MySQL support delimiters we can simply add those in order for it to work, but in Postgres we can use the standard schema syntax.

If you consider that when most database utilities export things they print the full identifier and that is solves our problem the easier way seems the best choice.

That means that database layer simply needs to do the following:

MySQL:

`tablename`

Postges:

public.tablename

If we really want to we can aways use the database name instead/addition:

MySQL:

dbname.tablename
dbname.`tablename`

Postges:

dbname.public.tablename

This can be implemented in all queries since the db layer already requires {} around table names.

Damien Tournoud’s picture

Status: Needs work » Closed (won't fix)

@jwuk: Consistent naming is useful for abstract CRUD operations as the title suggest. It would open up things like drupal_save($object); Where object has a type of 'user', or 'node'. Wouldn't make sense to have a type of 'users' as it only contains a single user. Not to mention this battle has been fought in many communities with the same result. It is like using all verbs or all nouns in menu item names...choose a standard and go with it.

This is completely unsubstantiated. As jwuk pointed out, it is *really simple* to have different table names then user names. Any ORM (even the most simple one) supports that. Plus, if we follow the route Crell and others have indicated, the name of our objects would probably more be DrupalUser and DrupalNode than just User and Node.

I have noticed a number of issues to make things more consistent. For instance the VERY minor appending (dot) syntax. That got in in the name of consistency. Having consistent tables name is a good thing and as I pointed out above, it points out a bigger problem.

Consistency is an overrated thing. Consistency for which use? At which cost?

If you consider that when most database utilities export things they print the full identifier and that is solves our problem the easier way seems the best choice.

Drupal strategy is "do not use words that are reserved in the major database engines". Please see #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements. As such, "users" can't become "user", because this last identifier is reserved in *both* MySQL and PostgreSQL.

boombatower’s picture

Status: Closed (won't fix) » Active

You ignored my most convincing argument...

Please try again.

@Damien Tournoud: Just because one database cannot handle a table name is by NO means proof that it is a bad concept. Instead we should consider the fact that both databases use the table name "user" for there table. I find your conclusion rather rash to put it frankly.

I think it really opens up a new issue of correctly and fully supporting Postgres. Contrib modules have no-one to tell them, "You can't use this table name because Postgres doesn't like it." So just as in previous version of Drupal the contrib will be able to "break support" for Postgres. If you are to fully support this you need a way to ensure that any table name in a module either: a) breaks on all database consistently, or b) works on all databases. Anything in between leaves room for error which if everyone is so adamant about supporting other databases we should take the time to do right, not quickly revert this change and stray from the main issue.

Secondly...great then lets use drupal_user, drupal_node. I still think the above solution for allowing all table names should be implemented regardless of what table names core uses and it should be noted that public.user works in Postgres....this isn't a DOES NOT WORK PERIOD as you seem to say.

Lastly just because it is a reserved word doesn't mean you can't use it...as is evident by Postgres working with it and MS SQL and MySQL providing delimiters.

Certain words such as SELECT, DELETE, or BIGINT are reserved and require special treatment for use as identifiers such as table and column names. This may also be true for the names of built-in functions.

Reserved words are permitted as identifiers if you quote them as described in Section 8.2, “Schema Object Names”:

http://dev.mysql.com/doc/refman/5.0/en/reserved-words.html

I am still fine with drupal_user, drupal_node...

Damien Tournoud’s picture

Status: Active » Closed (won't fix)

You ignored my most convincing argument...

... and you are quoting yourself saying: "Just because one database cannot handle a table name is by NO means proof that it is a bad concept"... which is far from even being an argument.

As I wrote in #52, the idea that we need to set all table names to singular to "support CRUD operations", is completely unsubstantiated. The truth is: there is no convincing reason for renaming all table names to singular, but there are *a lot* for no renaming them.

Renaming has a huge cost (for core *and* contrib developers), while not having any sensible benefit. Won't fixing again. This is a completely useless crusade.

boombatower’s picture

Status: Closed (won't fix) » Active

That is the first line of two paragraphs and is in response to you, "There is no way we convert users to user. And this proves my points in #25 and #29: this renaming is a complete waste of everyone's time. This issue should never have been opened."

Not an argument but disproving yours...

Regardless please read the rest of the paragraphs as the issue lies in truly supporting postgres in contrib and such.

It should also be explicitly noted that public.user works.

Damien Tournoud’s picture

Status: Active » Closed (won't fix)

Jimmy, please read (in detail) #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements, and return here afterward.

To sum up:

(1) There is no point in renaming the tables, because any (even trivial) object loading implementation support mapping class names to table names
(2) In 371, we concluded that our strategy is not to use any reserved word in major db engines

Conclusion: won't fix.

boombatower’s picture

Status: Closed (won't fix) » Active

My point is it doesn't matter what name core uses....no one will be there to stop contrib...so it would make sense to "If you are to fully support this you need a way to ensure that any table name in a module either: a) breaks on all database consistently, or b) works on all databases. Anything in between leaves room for error which if everyone is so adamant about supporting other databases we should take the time to do right, not quickly revert this change and stray from the main issue."

Otherwise you open up the possiblity for not supporting all dbs in all modules...considering how easy the fix is...

I'm not really as concerned about CRUD as I am about consistency...

You wouldn't name an object DBQuerys, you would call it DBQuery...just a fact-o-life...so if we need to use drupal_user so be it.

Damien Tournoud’s picture

My point is it doesn't matter what name core uses....no one will be there to stop contrib...so it would make sense to "If you are to fully support this you need a way to ensure that any table name in a module either: a) breaks on all database consistently, or b) works on all databases. Anything in between leaves room for error which if everyone is so adamant about supporting other databases we should take the time to do right, not quickly revert this change and stray from the main issue."

Otherwise you open up the possiblity for not supporting all dbs in all modules...considering how easy the fix is...

This discussion has nothing to do in that issue. Please don't reopen it for that, but discuss on #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements. If it was easy to fix, I guess it would have been fixed long ago (please see the discussion in the other issue, as I kindly asked you to already).

I'm not really as concerned about CRUD as I am about consistency...

You wouldn't name an object DBQuerys, you would call it DBQuery...just a fact-o-life...so if we need to use drupal_user so be it.

Consistency to what benefit? As I told you already, renaming has a huge cost (it forces contrib module maintainer to change all the queries, cause conflicts in patches for core, breaks the bootstrap during upgrade and force us to do ugly things in a pre-upgrade function, etc.). There is no compeling argument for the consistency, and lots of then against it.

boombatower’s picture

I'll try to spend time reading the whole issue...but from what I did read it seems like what I wrote makes sense...

I would not worry about modify maintainers having to run a simple grep. This by far is the easier thing to upgrade, not to mention the work being to to make it automatic.

David_Rothstein’s picture

Subscribe.

It seems like the particular problem with {users} -> {user} should be dealt with over at #330983: Rename user module tables to singular and this issue should be left for the more general discussion.

Also, the general approach for using update_prepare_d7_bootstrap() for the table renaming needs more thought because it happens too early in the update.php page load to be safe -- see the discussion about that at #330983: Rename user module tables to singular as well....

David Strauss’s picture

Priority: Critical » Normal

Regardless of how this discussion goes forward, this issue is most certainly not "critical."

Josh Waihi’s picture

Category: task » bug
Priority: Normal » Critical
Issue tags: +PostgreSQL Surge

@David Strauss, how is this not critical? A database the Drupal supports, doesn't install at the moment because of this issue. This is serious. Why does Drupal not make a principal of not using SQL reserved words, then this just wouldn't happen.

@boombatower: using alternative syntax like public.table or `table` in MySQL is not ideal and would break things if we wanted to use drupal over schema or databases (MySQL). At the moment, it feels very much like "We're the MySQL community and don't care if it doesn't work on your database" which is crap because I've (as have other non-MySQL db maintainers) written many fixes and enhancements for Drupal and ensured they worked on MySQL. With D7, we've seperated things out to encourage contrib developers to use the database layer rather than executing direct SQL so if anything, we've made it easier for contrib developers to make modules that work against all databases. But this is a valid point, maybe some guide lines need to be drawn up to ensure that everyone develops with all databases in mind rather than just MySQL. This will be better once PIFR starts running the postgres support on d.o

@Josh Waihi: It makes no sense that SELECT * FROM user would return the database's users as they are in a different database information_schema. Instead I would expect that to result in a syntax error.

'SELECT * FROM user' returns the users who are currently connected to the database. Regardless of weather you think it makes no sense or not, it is the case.

The fact that a table in Drupal conflict with a database Drupal supports should be enough to fix the table, in this case rename it. I'm with Damian on this in the respect that making it work is more important than a naming convention and a database driver shouldn't have to alter itself for that sake.

promoting this to the surge in hopes we can get some more light on this.

nedjo’s picture

It was indeed a failure on the part of those of us advocating this change not to recognize that thorough table naming consistency was not going to be possible given the issue of reserved words.

Since we've already decided against some options in #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements, I see two possible approaches (barring reopening that discussion):

1. Abandon the aim of table name consistency and roll back some or all of the patches applied so far (at least the user patch).

2. Introduce a table naming convention that either:

(a) uses a namespace (e.g., 'drupal_') for all tables to prevent collisions or

(b) provides a namespace or other fallback to use in the case that a table name would collide with a reserved word.

It's true that, if we were to introduce an ORM-style object-name to table-name mapping, this could help address our issues here in that developers might be shielded from having to remember inconsistent table names. But I don't know of a patch on the way that would introduce such a mapping. And, if at some point we get one, increased consistency in table names still wouldn't hurt.

2.a seems excessive--there's no good reason to rename all tables. I think 2.b, provide a fallback to use when a singular table name would collide with a reserved word, is worth some thought.

Josh Waihi’s picture

Category: bug » task
Priority: Critical » Normal

Mybad, PostgreSQL can quote tables "user" works, however this will need to be done on all tables. Maybe its time that Drupal starts quoting tablenames over all database since we can't tell what databases will reserve which words in the future. Few links to look at http://en.wikibooks.org/wiki/SQL_dialects_reference/Data_structure_defin... and http://www.alberton.info/dbms_identifiers_and_case_sensitivity.html.

I don't think this issue is a place to discuss this however so continuing database quoting discussion over at #370240: PDO should quote index names in table declarations.

Edit: Actually, better to discuss at #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements

boombatower’s picture

#64: That was my suggestion in #51 is that the database layer simply appends something to the tables to allow them to work consistently. If "" works then it would seem the problem is solved.

Replace {tablename} with `prefixtablename` or "prefixtablename" or whatever based on the database backend. Otherwise if we still get discontent then perhaps the db layer should just auto-namespace everything as suggestion in #63.

The above two solutions seem like the best ones as both allow consistent table names while supporting all databases.

Josh Waihi’s picture

@boombatower: you're still ignoring the fact that it has been agreed to not use reserved words, also, using "" breaks alot of other things in the database layer like sharing tables across drupal instances. You're comments come across very "lets hack it to make it work". At the moment we can use db_prefix like:

$db_url = array(
'default'   => '',
     'users'     => 'shared.',
     'sessions'  => 'shared.',
     'role'      => 'shared.',
     'authmap'   => 'shared.',
     'sequences' => 'shared.',
);

In MySQL, 'shared' would be another database, in PostgreSQL, a schema. I use this to share user credentials among common sites. And is cleaner than prefixing with 'shared_'. If we introduced quoting with prefixing, it would break. Where, in this context, users would translate to shared.users, with quoting, that would come out as "shared.users" which is a syntax error. should be shared."users" or shared.`users`. BTW, MySQL can support real quotes as per the SQL standard.

The database layer should not have to alter like this just so we can use a reserved word, that, IMO, is not adhering to previous discussion and rendering efforts like #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements pointless.

We should be building Drupal's architecture to work with the supported databases and not make the databases work for Drupal. Its quiet apparent that this isn't the case at the moment; for example we don't have datetime support implemented. And this is another step in the wrong direction. So can we at least revert untill this is sorted, because not being able to install HEAD means I can't work on other issues untill this is resolved.

Motion to rollback.

David Strauss’s picture

@Josh Waihi It's not "critical" because this issue is a task titled "Consistent table names to facilitate CRUD operations." If that's not accurate, update it or create another issue. I don't support using "critical" priority ratings for "Oh no, this issue broke HEAD!" It's useless in the issue listings, and it remains confusing unless someone reads through all the issue comments.

Having PDO escape/quote table names isn't "lets hack it to make it work," it's properly handling the issue that commands and table names both happen in the same (string-based) information band. Escaping table names is the right thing to do, and we should do it even if we can conveniently tiptoe around reserved words.

As for sharing tables, the way you're suggesting is cumbersome, and I'd argue we shouldn't even support it. If you want to share tables in MySQL, just create a "SELECT * FROM [table]" VIEW in the target Drupal database for the [table] you want to share. It's read+write, and Drupal doesn't even have to know it's sharing the table. It's also faster and supports nice MySQL VIEW permissions. Now, the actual shared table needs to be directly used by one Drupal instance if you want automatic schema updates to work, but you're already in a mess with schema updates when you use table sharing. Also, I checked, and PostgreSQL also supports writable VIEWs.

If we can't prefix tables once we start escaping them, that's a bug/limitation-to-be-fixed in the prefixing system, not an argument against escaping table names.

Finally, I can't support rolling back a patch just because it breaks PostgreSQL, especially because there's a clear path forward for fixing the issue.

Damien Tournoud’s picture

Assigned: boombatower » Unassigned

@David: I don't see where you see your "clear path forward". #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements has been opened with 2002, and the general agreement (including chx and Crell) was *against* table prefixing.

In my point of view, we now have a broken HEAD on at least one of our supported database (SQLite is also broken, but that's a different issue), so HEAD is broken. And we have no clear path forward.

dww’s picture

The short-term clear path forward is obvious: #330983-35: Rename user module tables to singular. Once that patch is reverted, we can revisit this (non-critical) task about trying to enforce one standard (singular tables) at the expense of another (don't use SQL reserved words) and try to find a compromise. Generally speaking, I agree with Damien -- there's a huge cost to this whole effort, with vanishingly small reward. The technical "reason" first given (facilitate CRUD) is mostly bogus -- there are other ways to handle that aspect. The "DX" benefits of all tables being singular are undone by the DX costs of conflicting with reserved words and the ensuing hoops we have to jump through for getting around that. I haven't seen any other reasons given to make all tables singular.

Whether or not we want to find a sane way to escape table names on all of our supported databases should be a discussion for another issue. Whether or not we should only support table prefixing via SQL VIEW is also orthogonal to this particular thread.

Everyone who's upset that HEAD is now broken on at least 2 of our "supported" DB engines and doesn't like that even visiting update.php will render your DB non-functional with D6 should comment over at #330983: Rename user module tables to singular, not here.

Thanks,
-Derek

David Strauss’s picture

The clear path forward is properly quoting table names, which ought to be easy given Schema API and the current requirement to "quote" table names with curly braces. There should be zero DX cost.

Josh Waihi’s picture

@David Strauss, for this topic, it is hacky. Discussing architectural database changes for the sake of CRUD operations is ridiculous. I firmly agree with Damian, dww, Crell and chx on this. By my count thats every db maintainer except you. Also, would you support a role back if it broke MySQL?

enough from me, I'm more concerned with getting HEAD fixed.

David Strauss’s picture

@Josh Waihi We're not talking about CRUD operations at this point. (And, personally, I see this as a DX issue.) We're talking about a choice of fixing how we handle table names or rolling back the patch. I still don't see how actually fixing the issue is hacky at all.

Also, would you support a role back if it broke MySQL?

Probably, but that's usually unnecessary because we actually test MySQL before committing patches. Breakage on MySQL is usually general breakage. If Drupal is going to support PostgreSQL and SQLite as first-class options, we need to set up CI test servers for those databases. Moreover, the vast majority of core developers are using MySQL (or have easy access to it), so it's not a strong development blocker to have non-MySQL databases broken in HEAD.

boombatower’s picture

Title: Consistent table names to facilitate CRUD operations » Consistent table names and database handling of table names

This has absolutely positively nothing to do with CRUD. This is not "hackish" as I have stated before consider the fact that contrib can use whatever name they please for a database table. It may then not work on postgres or mysql or w/e. The database layer should handle the names properly or throw an error if it is a reserved word. The latter being much uglier and you will have to maintain a list of reserved words. This is thus necessary and the result is not diminishing DX returns it is TRUE support of the 3 (+) databases in contrib as well.

This issue just uncovered the fact that the database layer has the problem. Please regard it as such, not a specific case of users vs user as everyone seems to be looking at it, or some ooohh ahhh CRUD specification. For some reason everyone picks on those two arguments and ignores the fact that this IS a problem (beating around the bush).

@Josh Waihi: If you read what I have written I want to genuinely support postgres and all other databases, but to truly do that changes need to be made.

Once we make this change then users can be named to w/e fits into the new schema, and if the database layer now correctly supports delimiting table names and it works then so be it.

dww’s picture

@boombatower:

A) No one is ignoring the question of table names using SQL reserved words. There's been an entire thread about that: #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements. The result of that thread was "proper escaping is Really Complicated(tm) in the general case, and it's better to not use reserved words". You seem keep ignoring that result. If you don't like that conclusion, please participate in that thread and address the points raised there.

B) We all agree CRUD is a red herring, thanks for removing that from the title.

C) Consistency in general is good. However, consistency is less important than avoiding reserved words. If the goal of consistency is to improve the developer experience, but the hoops developers must jump through to write their queries in such a way that columns and tables can be properly escaped is more of a PITA, then the change is a net loss for the developer's experience. In the abstract, it'd be great if the database layer could handle this and no one ever had to think about it. In practice, that's complicated. See #371.

D) "Everyone" is looking at {users} vs. {user} because in the absence of a solution to #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements, #330983: Rename user module tables to singular broke core's support for 2 "supported" database engines. It's not okay to leave those broken while we argue about #371. If/when we come to agreement on how to solve that issue, we can revisit {users}. Meanwhile, we need to get HEAD working again so that all sqlite and pgsql development doesn't grind to a halt, and we make even more work for ourselves.

David Strauss’s picture

There's a false premise in play here: that quoting table names also requires the major code overhauls to support quoting column names. But this thread is about table names only, and I would happily quote table names but forbid reserved words for column names. We shouldn't fight progress just because it's not a 100% solution.

dww’s picture

@David Strauss: Even quoting table names isn't trivial, as Josh Waihi points out in #66 above. Your answer was "we shouldn't let you do that, anyway", which I find unsatisfactory. Your alternative solution is to require yet more DB-specific functionality (writable VIEWs), with the classic "I know how to get this working on MySQL, I think it can work in PgSQL too, the rest of you: you're on your own" mentality. I don't find that satisfactory, either. ;)

I still don't see what this massive effort is buying us in practical terms for the benefit of Drupal, its users, or its developers.

David Strauss’s picture

@dww

* Updatable views is part of the SQL-92 standard
* SQL Server and Oracle also support writable views
* SQLite does not support writable views, but the #66 use case does not even apply to it because SQLite is database-per-file and requires ATTACH DATABASE, which we don't support, to use the additional, shared database.
* The prefixing method in #66 is more DB server-specific because the database.table/database.schema.table notation isn't even consistent across MySQL and PostgreSQL (as the example showed).
* The updatable VIEW approach continues to be faster.

David Strauss’s picture

I still don't see what this massive effort is buying us in practical terms for the benefit of Drupal, its users, or its developers.

It's not a massive effort. The patches have been written to quote table names in #371. We just have to apply them without any column or other identifier support, which is where the argument and controversy were.

The gain is that developers will not continue to write queries using the wrong singular/plural version of table names only to find out that the tables don't exist, having to run SHOW TABLES in the database, and having to go back to the code to fix it. It's bad DX, and the problem further spreads throughout contrib where, because core doesn't take a stand, all the table names are even more inconsistent in their naming. With lots of contrib modules installed, writing queries becomes a constant effort of listing table names in the database, even if you know the basic table names.

If we standardize core 100% on singular names and remove the reserved word barrier to some tables getting renamed, we can consider plural table names "bugs" and move toward consistency.

boombatower’s picture

David Strauss has stated it exactly. We only need table names escaped, reserved words for columns is a different issue. This requires a very minimal patch as the db layer already requires {tablename} in queries and everywhere else it just knows db_update('tablename'...); The standardization is always a good idea...and this does not require a lot of work.

This has been the idea all along, no need to exaggerate it.

webchick’s picture

Reading through the discussion here...

It's true that the outcome of #371: resolve ANSI SQL-92/99/2003 reserved words conflict in query statements was "don't use reserved names in table/column names" This is because reserved names can occur in either table names *or* column names. In order to catch both of those, you need to change every single query from:

-  $sql = 'SELECT DISTINCT u.uid, u.name, u.status, u.created, u.access FROM {users} u LEFT JOIN {users_roles} ur ON u.uid = ur.uid WHERE u.uid != 0';

to:

+  $sql = 'SELECT DISTINCT u.[uid], u.[name], u.[status], u.[created], u.[access] FROM [{users}] u LEFT JOIN [{users_roles}] ur ON u.[uid] = ur.[uid] WHERE u.[uid] <> 0';

And just... no. :P This is horrendous from a DX perspective, as it continues to enforce pain on module developers every. single. query. they write.

However, we are already tokenizing our table names in {}. All queries are run through prefixTables (in includes/database/database.inc). It seems to me as though it would be fairly trivial to have prefixTables call a makeSafeTables() or whatever, with code specific to each database system's way of quoting (`` in MySQL, "" in PostgreSQL, whatever in etc.). And this would represent a very nice DX improvement of module developers not needing to memorize the list of reserved table names.

Could someone explain to me why this isn't a useful thing to do? I saw reference to #66, but if #66 works with the existing table prefix code, I don't see a reason it wouldn't work with this.

Josh Waihi’s picture

@webchick, I've tried this, initially I thought that could be a solution but I ran into a couple of problems. Consider this:

$db_prefix = array(
    'default' => 'test_',
    'user'  => 'common.',
    // and other prefixes that don't matter for this example
);

For PostgreSQL, this means user would exist in another schema and in MySQL, another database.
Then take this query:
SELECT u.uid, u.name, n.title FROM {user} u INNER JOIN {node} n ON n.uid=u.uid;

If we use {} to add the quotes in, then user would become "common.user" and node would be "test_node". That breaks functionality because you would wrap quotes around a database/schema. Instead it should be common."user".

Secondly, we use {} in more places than just tablenames, we also use them in index definitions:

// line 550 of includes/database/pgsql/schema.inc
$query = 'CREATE INDEX {' . $table . '}_' . $name . '_idx ON {' . $table . '} (';

sqlite uses this too.

And as far as I've seen, no one has addressed either of those issues. As yet, Drupal's prefixing system, IMO, isn't robust enough to handle quoting tables.

@David Strauss: PostgreSQL testing servers are on their way, I've been working with boombatower to make that possible. It is really hard for those who support/prefer PostgreSQL to keep track of what is going into HEAD and to ensure it works on PostgreSQL. I stop watching for 2 weeks and came back to a broken HEAD again. If Drupal is going to support PostgreSQL and SQLite as A-class options (as you said), it starts with the rest of the community, the MySQL community, to reckonise us as such and help us resolve problems when they arise as we do for you. Just because we're a minority doesn't mean we can be pushed around to 'fit in with Drupal'. We're apart of it just as much as anyone else.

David Strauss’s picture

@Josh Waihi I still don't see why writable VIEWs aren't at least as effective for what you want to do.

Josh Waihi’s picture

@David Strauss can you give me an example?

David Strauss’s picture

@Josh Waihi Let's say you have three databases: drupal1, drupal2, and shared. You want to share the user table between them. You can put the user table in the shared database, drop the user table in drupal1 and drupal2, and add the view "user" to drupal1 and drupal2.

Here's how you would create the view in drupal1 and drupal2 (the syntax is the same for MySQL and PostgreSQL):
CREATE VIEW user AS SELECT * FROM shared.user;

That allows the sites running off drupal1 and drupal2 to transparently use the shared table without any awareness by Drupal.

It's slightly more complicated in PostgreSQL because you also need to create RULEs that tell PostgreSQL to redirect INSERT, UPDATE, and DELETE to the shared table, but you end up with the same effect with no involvement by Drupal.

boombatower’s picture

#81: Your example is that of "reverse db support" you want to support a specific feature of postgres that isn't supported across the board and is an example of a "hack". I don't see why the system should have to support that through the "hack".

The same as me arguing that we should support the mysql feature that user works as a table name...doesn't make a sound argument.

Josh Waihi’s picture

I thought this was a feature, not a hack. If this isn't the case then I'm ok with losing the ability for the greater good. What @david strauss explains with views seems fine to me.

Any suggestions on how to get around the index quoting? or since '{' doesn't get used (yet) do we just move:
'CREATE INDEX {' . $table . '}_' . $name . '_idx ON {' . $table . '}
becomes:
'CREATE INDEX {' . $table . '_' . $name . '_idx} ON {' . $table . '}

I'm assuming its ok to quote indexes

Josh Waihi’s picture

In regards to @boombatower's comment about the 'hack'; how can it be a hack if it is documented in the Code:

  /**
   * Append a database prefix to all tables in a query.
   *
   * Queries sent to Drupal should wrap all table names in curly brackets. This
   * function searches for this syntax and adds Drupal's table prefix to all
   * tables, allowing Drupal to coexist with other systems in the same database
   * and/or schema if necessary.
   *
   * @param $sql
   *   A string containing a partial or entire SQL query.
   * @return
   *   The properly-prefixed string.
   */
  public function prefixTables($sql) {
    global $db_prefix;

    if (is_array($db_prefix)) {
      if (array_key_exists('default', $db_prefix)) {
        $tmp = $db_prefix;
        unset($tmp['default']);
        foreach ($tmp as $key => $val) {
          $sql = strtr($sql, array('{' . $key . '}' => $val . $key));
        }
        return strtr($sql, array('{' => $db_prefix['default'] , '}' => ''));
      }
      else {
        foreach ($db_prefix as $key => $val) {
          $sql = strtr($sql, array('{' . $key . '}' => $val . $key));
        }
        return strtr($sql, array('{' => '' , '}' => ''));
      }
    }
    else {
      return strtr($sql, array('{' => $db_prefix , '}' => ''));
    }
  }
boombatower’s picture

The other solution then if we want to continue supporting the PostgreSQL schema feature is to prefix with database....which seems just as correct and should allow for schema.

databasename.tablename

so then if prefix: "schema."

databasename.schema.tablename

or normal: "prefix'

databasename.prefixtablename

Josh Waihi’s picture

Josh Waihi’s picture

Status: Active » Needs work

@boombatower, not sure what you mean... the dot syntax MySQL uses is wrong. it should be database.schema.table but MySQL doesn't utilise schemas, though the column is there for them to use in the information_schema.

I wrote some stuff about that patch but it didn't submit, I think because @boombatower had posted whilest I was writting. basically, it works on MySQL but it doesn't work on PostgreSQL. for some reason, it turns the last insert id functionality off on the pdo driver for PostgreSQL. Would be nice to get some help on the situation. In the PHP manual there is a comment that the pdo driver for postgresql, just does a SELECT CURRVAL(sequence_name); to return the last insert id so maybe we just do that manually?

boombatower’s picture

I understand, but if this is already database specific "prefixing" or w/e then it could be done, but if you are alright with "quoting" that is by far simpler.

dww’s picture

@Josh Waihi (off topic): the bug you encountered is probably this one: #379470: Change in the issue description is discarded after clicking on the attach button

Damien Tournoud’s picture

Of course, if we want to encode identifiers, we have to do it properly. If $db_prefix = 'myschema.', which is supported on *both* MySQL and PostgreSQL (MySQL just calls a "schema" a "database", for historical reasons), the resulting query should be (using MySQL quoting syntax):

SELECT u.uid FROM `myschema`.`user` u WHERE u.uid = 1

David Strauss’s picture

@Damien Tournoud I think the argument here is to either prefix with the db/schema or quote the table identifiers. Either will solve the user table problem on PostgreSQL, and either represents progress from the current code.

Josh Waihi’s picture

Status: Needs work » Needs review

I'm for using dot syntax rather than quotes as at this stage it seems more elegant in PostgreSQL and easier to understand than using views.
changing the status to test the patch above

Status: Needs review » Needs work

The last submitted patch failed testing.

Josh Waihi’s picture

Status: Needs work » Active

I see this schema referencing thing going into its own and seperating from db_prefix altogether. If we're going to do this properly, lets use db_prefix as it was intended, so multiple instances of drupal can be installed on a single database for those with hostingplans that give one database. And lets implement a feature to support schemas in PostgreSQL and databases in MySQL. We already have methods in place to make the implementation fairly smooth.

I would imagine it would be implemented very similar to db_prefix but without the hassle of figuring out if it is a db_prefix or schema.

If I can get a second ok on this, I'll code up patch.

boombatower’s picture

+1

boombatower’s picture

@Josh Waihi: any progress?

Josh Waihi’s picture

Status: Active » Postponed

@boombatower, been dicussing this Crell and ahve some other things to think about. Also prefixes are about to go into core on a per connection bases which will alter this as well. gonna wait for that first I think, however I think it alters the install so it's waiting on PIFR which is waiting on installer in core.

back to you ;p

boombatower’s picture

hehe...I love chains of dependencies.

drewish’s picture

Version: 7.x-dev » 8.x-dev
Status: Postponed » Needs review
mki’s picture

-1 for this

  1. Please read "Joe Celko's SQL programming style" where he agrues in very interesting way that:
    1. Collective or class table names are better than singular names [...] [1]
    2. And also chapter 3.15 titled "Do Not Use Object-Oriented Design for an RDBMS" [2]
  2. Practical reason: please take any book with tables, then look at table names (captions) and column names. Which is usally singular/plural?

----
[1] http://books.google.pl/books?id=90c41yKz3IUC&pg=PA14&dq=%22Collective+or...
[2] http://books.google.pl/books?id=90c41yKz3IUC&pg=PA14&dq=%22Collective+or...

boombatower’s picture

users
users_types

vs

user
user_type

In extremely simple cases like 'users' by itself you can get away without any weirdness, but a user has a type not users types that is just plain bad. I never like to argue that because a way is popular that makes it right...that never gets you anywhere and there are loads of examples of that.

I don't expect this to go anywhere although I think it should. My primary concern is that drupal is consistent even if not using the singular case, currently it uses both.

blocked_ips
users

vs

block
block_role
block_custom
...

instead of blocks, blocks_roles

which again leads you to, a block has a role(s) so if anything to be the way we would write it in a sentence you would have: blocks and block_roles the plural always being at the end. But that is bad since the detail records don't contain the base table name, so again the easy way to solve....get rid of the plural.

mki’s picture

@boombatower, please take into account that this is not only table naming problem. For example $block_roles variabie makes more sense than $block_role, if this variabie is an array that have many data elements. It is not logical to query "block_role" table and store the result in $block_roles.

So +1 for consistent naming, but -1 for singular table names.

boombatower’s picture

Again, we don't have block_roles right now...we have either blocks_roles or block_role currently. No were do we have the standard English way of adding the plural s to the end. As I pointed out doing so is problematic since you will have blocks and block_roles which I guess if fine, but not having the base table in the detail table name seems silly and if we go all plural then we don't have the standard variable name used either, instead we end up with blocks_roles. So rather then come up with a bunch of silly naming rules dropping the s altogether is much simpler and consistent, but again I'm fine with something else just needs to be consistent which it is not currently.

Chris Johnson’s picture

One problem with using plural English nouns for tables is outlined in the following blog: http://sqlsoundings.blogspot.com/2008/11/table-names-should-be-singular.... English plurals are inconsistent (as are most languages), and we often use collective nouns (equipment -- what's the singular) instead of plurals.

Relational databases were built upon a branch of mathematics called relational algebra. Tables were not originally called tables, but "relations" and rows in a table were called "tuples". Hence, by definition a relation was singular, not plural. That is, there was only one specific relation between the attributes in the tuple described in a single relation or table -- not many.

So historically, database tables were always correctly named with singular names, because the "table" described a single relation, rather than current popular thinking that a table is something that contains multiple things, and therefore should be plural.

Now that we have the facts out of the way, there's another reality to consider: popular usage, no matter how wrong, often wins out over, or eventually supersedes correct usage, in many fields, especially in natural languages.

Since I'm an old-school database programmer, I'm in favor of singular names -- and it means I make fewer typos since I have to type fewer characters. ;-)

But what's really important is just plain consistency, so we don't make mental "typos" as we try to remember what someone named a table.

mki’s picture

@boombatower, @Chris Johnson – we have to remember that programming languages are artificial languages. Unfortunately, we are choosing identifiers that are expressed in English which is natural language. So all these efforts to make English behaving like some formalized language (by removing or simplifying plural forms) are pointless. No one really going to remove all English complexity from code base, not to mention Doxygen documentation. It is a programmer duty to know some English basics or have English dictionary. As long as we are using English, we should use it as usual, that is with plural forms.

@Chris Johnson: "[...] "table" described a single relation, rather than current popular thinking that a table is something that contains multiple things, and therefore should be plural."

I would say that it is current popular thinking that given table is a description of data, or data structure, or class, or schema. It is not. Tables are structured data, not just data structure, data description, class, schema and the like. Table = data = rows. When row = 'user', then table = 'users'. Keep it simple, but not simpler than it is.

boombatower’s picture

Not only does that not really refute #107, but the whole lets leave english in there is great, but then we should use users, and user_roles which we currently use users_roles...haven't really said a whole lot. You don't see java naming their string class Strings just because you can have lots of them. As stated in #107 the string defines a structure/relation one thing that you can instantiate many times just like rows being instances of a class...which any db to class mapping system out there would agree with and the theory seems to as well. So if I were to map the users table to a class I wouldn't want to call it Users I would want to call it User, because that's what a row represents.

The fact of that matter is the current drupal database is inconsistent. Not sure if we can just take a poll or have one the db guys (say Crell) pick one, but we just need a standard. I think the evidence is compelling, but oh well. So I'm open to how we can resolve this one way or another.

mki’s picture

@boombatower, SQL is something different than object-oriented programming (think of object database instead). So SQL and OOP should not be confused, even if one can think of SQL table or row in object-oriented terms, this is just 'mapping'. Anyone can use SQL with almost every modern programming language and programming paradigm. And Drupal actually does it, since Drupal is not mostly object-oriented, just like PHP isn't object oriented programming language.

I'm off this issue at this time, but let my point of view stay on the table to the final decision. Thanks for considering this and discussion.

Liam Morland’s picture

Issue tags: +PostgreSQL

Tagging

jhedstrom’s picture

Version: 8.0.x-dev » 9.x-dev
Status: Needs review » Needs work

I think it's too late for this one to make it into 8.0, or even 8.x since it would be fairly disruptive. Bumping to 9.

catch’s picture

Status: Needs work » Postponed (maintainer needs more info)

Singular table names proved to be impossible given SQL reserved words.

So this is 'needs more info' for me.

Also SQL table names are no longer part of the API - people shouldn't be querying them directly anyway.

xjm’s picture

Version: 9.x-dev » 8.8.x-dev
Status: Postponed (maintainer needs more info) » Closed (won't fix)

I agree; I think this should be considered wontfix given Drupal 8's architecture.

Liam Morland’s picture

Even if we don't rename any tables, there should be a coding standard pointing to how new tables should be named.