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

Fixed usage of undefined variable in empty() expression in SqlWalker #6621

Merged
merged 1 commit into from
Aug 13, 2017
Merged

Fixed usage of undefined variable in empty() expression in SqlWalker #6621

merged 1 commit into from
Aug 13, 2017

Conversation

Powerhamster
Copy link
Contributor

changed $conditions to $condition so $isConditionalJoin is working

changed $conditions to $condition so $isConditionalJoin is working
@Ocramius
Copy link
Member

I must say that I have issues wrapping my head around the fact that builds fail on 2.5 and passed in this patch: at this point, it must be a caching issue.

Still, this is why you don't use empty(), kids!

Thanks @Powerhamster, I'm doing a fixup release with this patch in it ASAP

@Ocramius Ocramius self-assigned this Aug 13, 2017
@Ocramius Ocramius added this to the 2.5.8 milestone Aug 13, 2017
@Ocramius Ocramius added the Bug label Aug 13, 2017
@Ocramius Ocramius merged commit c83f479 into doctrine:2.5 Aug 13, 2017
@Ocramius Ocramius changed the title fixed undefined variable Fixed usage of undefined variable in empty() expression in SqlWalker Aug 13, 2017
@Ocramius
Copy link
Member

Related:

#6464
#6475

This bug was introduced by an incorrect backport performed by me.

@Powerhamster Powerhamster deleted the patch-1 branch August 13, 2017 19:43
@@ -1143,7 +1143,7 @@ public function walkJoin($join)
$dqlAlias = $joinDeclaration->aliasIdentificationVariable;
$tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias);
$condition = '(' . $this->walkConditionalExpression($join->conditionalExpression) . ')';
$isUnconditionalJoin = empty($conditions);
$isUnconditionalJoin = empty($condition);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this be always true given the previous line always add brackets to the $condition variable meaning it will never be empty ?

Copy link
Member

Choose a reason for hiding this comment

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

@jvasseur yes, that's not an issue though, as the code was cleaned up on master

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I try to fix it?

Copy link
Member

Choose a reason for hiding this comment

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

@Powerhamster no - ORM 2.5.8 is green and doesn't need improvement here.

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.

3 participants