Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Get correct expression string when Zend\Db\Sql\Expression is used #7422

Conversation

Martin-P
Copy link
Contributor

@Martin-P Martin-P commented Apr 4, 2015

Fix for #7407.

The processing of Zend\Db\Sql\Expression assumed the value of $part was the correct value to be used, however when an instance of Zend\Db\Sql\Expression is provided the correct value is in Zend\Db\Sql\Expression::getExpression().

@Martin-P Martin-P force-pushed the hotfix/abstract-sql-process-expression branch from 4f3f974 to c8e85ea Compare April 4, 2015 21:30
@@ -108,6 +108,13 @@ protected function processExpression(ExpressionInterface $expression, PlatformIn
$expressionParamIndex = &$this->instanceParameterIndex[$namedParameterPrefix];

foreach ($parts as $part) {
// #7407: use $expression->getExpression() to get the unescaped version of the expression
if (is_string($part) && $expression instanceof Expression) {
Copy link
Contributor

Choose a reason for hiding this comment

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

May be it'll be better:

if (is_string($part)) {
    if ($expression instanceof Expression) {
        // #7407: use $expression->getExpression() to get the unescaped version of the expression
        $sql .= $expression->getExpression();
    } else {
        // simply tack it onto the return sql "specification" string
        $sql .= $part;
    }
} else (is_array($part)) {
    //
} else {
    throw // Elements returned from getExpressionData() array must be a string or array.
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also possible, but I tried to avoid nesting if.

Copy link
Contributor

Choose a reason for hiding this comment

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

To me It looks better then continue and we get rid of throw in a middle of a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When looking at different pull requests I often see the preferred way is to use continue instead of if/elseif/else structures. Personally I find the continue easier to read, but it is all just a matter of preference.

For now I will leave it as it is, but if the ZF2 maintainers want the other structure it is no problem and I can change it at that moment.

weierophinney added a commit that referenced this pull request Apr 8, 2015
…pression

Get correct expression string when Zend\Db\Sql\Expression is used
weierophinney added a commit that referenced this pull request Apr 8, 2015
@weierophinney weierophinney merged commit c8e85ea into zendframework:master Apr 8, 2015
weierophinney added a commit that referenced this pull request Apr 8, 2015
@weierophinney weierophinney added this to the 2.4.1 milestone Apr 8, 2015
@Martin-P Martin-P deleted the hotfix/abstract-sql-process-expression branch April 8, 2015 19:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants