Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Bugfix for join equality when default join type is used #638

Merged
merged 1 commit into from

3 participants

@arvenil

Since http://trac.propelorm.org/changeset/1959#file1 there is a bug in checking equality between two joins if you skip joinType parameter so below won't work:

$j1 = new Join('foo', 'bar');
$j2 = new Join('foo', 'bar', 'INNER JOIN');
$this->assertTrue($j1->equals($j2));

The change to bugfix this is simple, I've also added tests for other, currently working test cases.

@arvenil arvenil Bugfix for join equality when default join type is used
$j1 = new Join('foo', 'bar');
$j2 = new Join('foo', 'bar', 'INNER JOIN');
$this->assertTrue($j1->equals($j2));
dcace44
@staabm
Collaborator

LGTM

@willdurand willdurand merged commit 6407158 into from
@willdurand
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 27, 2013
  1. @arvenil

    Bugfix for join equality when default join type is used

    arvenil authored
    $j1 = new Join('foo', 'bar');
    $j2 = new Join('foo', 'bar', 'INNER JOIN');
    $this->assertTrue($j1->equals($j2));
This page is out of date. Refresh to see the latest.
View
5 runtime/lib/query/Join.php
@@ -226,8 +226,7 @@ public function setJoinType($joinType = null)
/**
* Get the join type
*
- * @return string The type of the join, i.e. Criteria::LEFT_JOIN(), ...,
- * or null for adding the join condition to the where Clause
+ * @return string The type of the join, i.e. Criteria::LEFT_JOIN()
*/
public function getJoinType()
{
@@ -548,7 +547,7 @@ public function equals($join)
{
return $join !== null
&& $join instanceof Join
- && $this->joinType == $join->getJoinType()
+ && $this->getJoinType() == $join->getJoinType()
&& $this->getConditions() == $join->getConditions();
}
View
22 test/testsuite/runtime/query/JoinTest.php
@@ -165,6 +165,28 @@ public function testCompositeeConstructor()
$this->assertEquals('LEFT JOIN', $j->getJoinType());
}
+ public function testEquality() {
+ $j1 = new Join('foo', 'bar', 'INNER JOIN');
+
+ $j2 = new Join('foo', 'bar', 'INNER JOIN');
+ $this->assertTrue($j1->equals($j2));
+
+ $j3 = new Join('foo', 'bar', 'LEFT JOIN');
+ $this->assertFalse($j1->equals($j3), 'INNER JOIN is not equal to LEFT JOIN');
+
+ $j4 = new Join('foo', 'bar', 'RIGHT JOIN');
+ $this->assertFalse($j1->equals($j4), 'INNER JOIN is not equal to RIGHT JOIN');
+
+ $j5 = new Join('foo', 'bar');
+ $j6 = new Join('foo', 'bar');
+ $this->assertTrue($j5->equals($j6), 'Joins without specified join type should be equal,
+ as they fallback to default join type');
+
+ $j7 = new Join('foo', 'bar', 'INNER JOIN');
+ $this->assertTrue($j5->equals($j7), 'Join without specified join type should be equal
+ to INNER JOIN, as it fallback to default join type');
+ }
+
public function testCountConditions()
{
$j = new Join();
Something went wrong with that request. Please try again.