Skip to content

Switching from Propel 1.6.9 to Propel 1.7.x and the possible bug in addMultipleJoin

I am dealing with an update of an application and we have some code inside that is using the "Propel::addMultipleJoin()" method. I know it is marked as deprecated but we are using it and it is still in there.
So we are creating a "Criteria" object and using the "addMultipleJoin". The "addMultipleJoin" method itself calls the "addJoinObject" which is calling the "equals" method of the join object (class "Join") for each already added join object. Now things starting to differ between the versions.

Version 1.7.x

The "addJoinObject" method is the following:


public function addJoinObject(Join $join)
{
    $isAlreadyAdded = false;
    foreach ($this->joins as $alreadyAddedJoin) {
        if ($join->equals($alreadyAddedJoin)) {
            $isAlreadyAdded = true;
            break;
        }
    }

    if (!$isAlreadyAdded) {
        $this->joins[] = $join;
    }

    return $this;
}

The "equals" method itself is checking the following criterias:

return $join !== null
    && $join instanceof Join
    && $this->getJoinType() == $join->getJoinType()
    && $this->getConditions() == $join->getConditions();

What have I done to debug this? I simple added the following code in "Join::equals()" right before the return statement and added a breakpoint to get the content out of it.

$joinParams = array();
$joinAsArray = array(
    'clauses' => $join->getClause($joinParams),
    'conditions' => $join->getConditions(),
    'join condition' => $join->getJoinCondition(),
    'join type' => $join->getJoinType(),
    'operators' => $join->getOperators(),
    'params' => $joinParams
);
$thisParams = array();
$thisJoinAsArray = array(
    'clauses' => $this->getClause($thisParams),
    'conditions' => $this->getConditions(),
    'join condition' => $this->getJoinCondition(),
    'join type' => $this->getJoinType(),
    'operators' => $this->getOperators(),
    'params' => $thisParams
);

Version 1.6.9

The "addJoinObject" method is the following:


public function addJoinObject(Join $join)
{
    if (!in_array($join, $this->joins)) { // compare equality, NOT identity
        $this->joins[] = $join;
    }

    return $this;
}

For your information, the class "Join" implements the magic "__toString()" method. Thats the fact why the "!in_array($join, $this->join)" is working.

public function toString()
{
    $params = array();

    return $this->getClause($params);
}

public function __toString()
{
    return $this->toString();
}

As you can see, in 1.6.9, the clauses where the base to validate if the join was already added or not. In 1.7.x, the condition (array) and the join type (e.g. "left join") are the base to validate if the join is already added or not. The problem with my statements is, that condition is always an empty array(), the join type is the same so they a marked as equal.

I digged through the changelog and each commit. Two are left, 93c6a0f and dcace44. I found a comment that is discussing this problem. So lets see. Funny sidenote, arvenil introduced this by fixing a bug (and i'm totally on his side, as long as code is inside, bugs should be fixed).

How to deal with that problem while no fix is available?
Currently, i made good process by doing the following replacement.


//broken code
$criteria->addMultipleJoin(
    array(
        array(
            MyTablePeer::COLUMN_ONE, MyOtherTable::COLUMN_TWO, Criteria::EQUAL
        ),
        array(
            MyTablePeer::COLUMN_TWO, 3, Criteria::EQUAL
        )
    ),
    Criteria::INNER_JOIN
);
//new code
$criteria->addJoin(MyTablePeer::COLUMN_ONE, MyOtherTable::COLUMN_TWO, Criteria::INNER_JOIN);
$criteria->add(MyTablePeer::COLUMN_TWO, 3, Criteria::EQUAL);

Taking the created sql statement and checking them via "EXPLAIN" is returning the same result and speed.

Update 2014-03-31

I did a pull request which got merged. Let's see if it passes the quality test and will be in the next release.

Translate to de es fr it pt ja

Trackbacks

No Trackbacks

Comments

Display comments as Linear | Threaded

No comments

The author does not allow comments to this entry

Add Comment

Standard emoticons like :-) and ;-) are converted to images.
E-Mail addresses will not be displayed and will only be used for E-Mail notifications.
To leave a comment you must approve it via e-mail, which will be sent to your address after submission.

To prevent automated Bots from commentspamming, please enter the string you see in the image below in the appropriate input box. Your comment will only be submitted if the strings match. Please ensure that your browser supports and accepts cookies, or your comment cannot be verified correctly.
CAPTCHA

Markdown format allowed
Form options