Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

1.3 TranslateBehaviour: Cleaner code and optimized query #199

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

bfanger commented Sep 12, 2011

Added "locale = $locale" to the ON part and changed the join type to an INNER join.
Because adding it to the WHERE part caused the translation to be mandatory anyway.
This cleans up the code and "INNER" joins are generally faster than LEFT joins.

@markstory markstory commented on the diff Sep 19, 2011

cake/libs/model/behaviors/translate.php
)
);
-
- if (is_string($query['conditions'])) {
- $query['conditions'] = $db->conditions($query['conditions'], true, false, $model) . ' AND '.$db->name('I18n__'.$field.'.locale').' = \''.$locale.'\'';
- } else {
- $query['conditions'][$db->name("I18n__{$field}.locale")] = $locale;
- }
@markstory

markstory Sep 19, 2011

Owner

Why are you removing support for string conditions? Unfortunately they are still supported.

@bfanger

bfanger Sep 20, 2011

Contributor

I'm not removing any conditions, string or otherwise, but because the $locale check is added to the conditions of the join(ON) there is no need to add it to the $query'conditions'.

@markstory

markstory Sep 20, 2011

Owner

Ah right, I missed that. I'll try applying the patch in a bit then :)

@bfanger bfanger Added the "locale = $locale" condition to the ON part of the query.
Added "locale = $locale" to the ON part and changed the join type to an INNER join.
Because adding it to the WHERE part caused the translation to be mandatory anyway.
This cleans up the code and "INNER" joins are generally faster than LEFT joins.
9f71268
Owner

lorenzo commented Oct 14, 2011

I'm not sure about the INNER change, that means that rows with no translation will not be returned, and I don't think that is the intended behavior

Contributor

bfanger commented Oct 14, 2011

Funny, that was my initial reaction also. but that IS the intended behavior.
The LEFT join i saw in the sql-log confused me because they are generally used for optional relations, but the "i18n.locale = x" part removes all the records where that value IS NULL from the resultset.

Owner

lorenzo commented Oct 15, 2011

Thanks for the patch and explanation, the change landed in [ab88f3e]

@lorenzo lorenzo closed this Oct 15, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment