Default SelectQuery::addField() alias to $field, not $table_$field [#316868]

As discussed in #314532: DBTNG comment.module, should we change the auto-aliasing logic that addField() uses? Currently if no alias is specified the default is $table_$field. Regardless, it will then check for a duplicate and append a counter to the alias name if necessary.

Since in most cases the field name is not a duplicate, should we make the default just $field, and if that is already in use then use $table_$field? That would simplify conversion as we wouldn't have to rename every single field when referenced and/or alias every single field when specified.

Thoughts?

Comment	File	Size	Author
#5	hook_file.patch	75.86 KB	drewish
#5
#5	file.php_.txt	4.61 KB	drewish
#4	select_alias.patch	2.28 KB	Crell
#4

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

merlinofchaos CreditAttribution: merlinofchaos commented 3 October 2008 at 22:05

The $table_$field bit was initially modelled after Views, but the situation here is a little different. Views queries really expect to have a lot of different unrelated hands poking at them, so $table_$field helps prevent collisions. In Drupal core, $table_$field does seem like a burden. Also, Views 2 cares a lot less about what the alias actually is, since it's a lot better at saying "add a field and tell me what the alias was". That's the critical part from the Views perspective: 1) it will always tell me the actual alias used and 2) it will always alias.

Comment #2

CorniI CreditAttribution: CorniI commented 4 October 2008 at 12:05

I'm definitly for defaulting the alias to $field, as I wrote in #314532: DBTNG comment.module. The return of addField can't be used really in core, as eg lot's of different places just expect the layout of a $comment-object, and they don't mind if there are aliases set, they'll just access a (maybe nonexistant) variable.

Comment #3

Crell CreditAttribution: Crell commented 5 October 2008 at 23:40

Well, as the docs say in that case it's better for them to not presume what the alias will be but simply use the value returned from addField() as the field name. That way even if the alias handling changes they will continue to work. Code that's doing that will work fine with a shorter default, however, so yeah, let's go ahead and change the default logic as described in the OP. ($field, then $table_$field, then $table_field_n)

Comment #4

Crell CreditAttribution: Crell commented 6 October 2008 at 01:27

Assigned:	Unassigned	» Crell
Status:	Active	» Needs review

File	Size
select_alias.patch	2.28 KB

And here's the patch to do that, including unit test updates for it.

Comment #5

drewish CreditAttribution: drewish commented 8 October 2008 at 23:25

File	Size
file.php_.txt	4.61 KB
hook_file.patch	75.86 KB

subscribing

Comment #6

Dave Reid

he/him

English

Nebraska USA

CreditAttribution: Dave Reid commented 18 October 2008 at 20:42

Status:

Needs review

» Reviewed & tested by the community

Patch makes sense, looks good, and applies with just a little fuzz. Had a few node revision/path translation test failures and exceptions, but they all happened on current HEAD as well, no fault to this patch.