Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that all database columns are properly aliased when generating sql :) #414

Closed
wants to merge 4 commits into from

Conversation

pnomolos
Copy link
Contributor

@pnomolos pnomolos commented Apr 2, 2012

No description provided.

@nateabele
Copy link
Member

This needs: (1) QA fixes, (2) a test, (3) to be submitted against master. :-)

@nateabele
Copy link
Member

Oh, sorry, I mis-spoke. It already was submitted against master, I meant dev.

@nateabele
Copy link
Member

Also, it looks like some extra commits snuck in there related to another issue.

@@ -564,6 +564,7 @@ public function conditions($conditions, $context, array $options = array()) {

foreach ($conditions as $key => $value) {
$schema[$key] = isset($schema[$key]) ? $schema[$key] : array();
$key = (strpos($key, '.') !== FALSE) ? $key : ($context->alias() . "." . $key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FALSE needs to be lowercase here and "." . $key should become ".{$key}".
Also we'll need a test case for this. Usually if you provide one along a suggested patch yourself, patches can get merged much quicker.

@nateabele
Copy link
Member

Okay, this is fixed by #482.

@nateabele nateabele closed this May 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants