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

Allow literal zero string as sql expression #7290

Closed
wants to merge 2 commits into from
Closed

Allow literal zero string as sql expression #7290

wants to merge 2 commits into from

Conversation

marcelto
Copy link
Contributor

@marcelto marcelto commented Mar 4, 2015

A sql expression cannot currently be a literal zero (useful in certain unions) since setExpression will never be called.

$select = new Select('foo');
$select->columns([
'bar' => new Expression('0') //fails, '0' is never set as the expression and the default empty string is used instead resulting in malformed sql
]);

A sql expression cannot currently be a literal zero (useful in certain unions) since setExpression will never be called.

$select = new Select('foo');
$select->columns([
   'bar' => new Expression('0') //fails, '0' is never set as the expression and the default empty string is used instead resulting in malformed sql
]);
@malukenho
Copy link
Member

Missing unit tests and fix build.

@weierophinney
Copy link
Member

@malukenho We have errors on the master branch right now due to a change in the Travis environment that causes 5.5 and 5.6 to error for Zend\Cache; the build errors are not related to this change.

However, @marcelto : we do need unit tests for this before I can accept it to merge.

@malukenho
Copy link
Member

thanks for the explanation @weierophinney

tests that the constructor will set the expression property even when
the expression is the string '0'
@marcelto
Copy link
Contributor Author

added new test case

weierophinney added a commit that referenced this pull request Mar 18, 2015
Allow literal zero string as sql expression
weierophinney added a commit that referenced this pull request Mar 18, 2015
@weierophinney
Copy link
Member

Merged to develop for release with 2.4.

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