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

Selection: added new method whereOr #111

Merged
merged 1 commit into from May 22, 2016
Merged

Selection: added new method whereOr #111

merged 1 commit into from May 22, 2016

Conversation

@meridius
Copy link
Contributor

meridius commented Dec 7, 2015

This feature will make creating the OR-ed where conditions easier by passing an (associative) array as its parameter.

@meridius meridius force-pushed the meridius:added-whereor branch 2 times, most recently from cd4211b to 23ad49e Dec 7, 2015
@meridius meridius changed the title added new method whereOr Selection: added new method whereOr Dec 7, 2015
@meridius meridius force-pushed the meridius:added-whereor branch from 23ad49e to fa7d0ed Dec 7, 2015
*/
public function whereOr(array $parameters)
{
if (count($parameters) >= 2) {

This comment has been minimized.

Copy link
@Majkl578

Majkl578 Dec 8, 2015

Contributor

If you invert this condition, you can simply avoid putting whole method body into one large condition.

} else {
$this->where($parameters);
}
return $this;

This comment has been minimized.

Copy link
@Majkl578

Majkl578 Dec 8, 2015

Contributor

Not needed, just forward-return where() call.

test(function () use ($context) {
$count = $context->table('book')->whereOr([
'translator_id IS NULL',
'title' => 'Dibi',

This comment has been minimized.

Copy link
@Majkl578

This comment has been minimized.

Copy link
@meridius

meridius Dec 8, 2015

Author Contributor

What's wrong here?

/**
* Adds where condition using the OR operator between parameters.
* More calls appends with AND.
* @param array array('column1' => 1, 'column2 > ?' => 2, 'full condition')

This comment has been minimized.

Copy link
@Majkl578

Majkl578 Dec 8, 2015

Contributor

Replace array() with [].

@meridius meridius force-pushed the meridius:added-whereor branch from fa7d0ed to 440577d Dec 8, 2015
foreach ($parameters as $key => $val) {
if (is_int($key)) { // whereOr(['full condition'])
$columns[] = $val;
} else if (strpos($key, "?") === FALSE) { // whereOr(['column1' => 1])

This comment has been minimized.

Copy link
@matej21

matej21 Dec 8, 2015

Contributor

else if => elseif

$values[] = $val;
} else { // whereOr(['column1 > ?' => 1])
$columns[] = $key;
$values[] = $val;

This comment has been minimized.

Copy link
@matej21

matej21 Dec 8, 2015

Contributor

I think, this should also support multiple parameters, e.g.
['SOME_FN(column, ?, ?)' => [1, 2]]

maybe this way?

$values = array_merge($values, substr_count($key, '?') > 1 ? $val : [$val]);
$values[] = $val;
}
}
$columnsString = implode(" OR ", $columns);

This comment has been minimized.

Copy link
@matej21

matej21 Dec 8, 2015

Contributor

I would surround every expression with braces:

$columnsString = '(' . implode(') OR (', $columns) . ')';

So you can use sth like this:
['foo AND bar > ?' => 1]

@meridius meridius force-pushed the meridius:added-whereor branch 2 times, most recently from fe5d58c to 881c210 Dec 9, 2015

/**
* Test: Nette\Database\Table: WhereOr operations
* @dataProvider? ../databases.ini mysql

This comment has been minimized.

Copy link
@meridius

meridius Dec 9, 2015

Author Contributor

Can anyone think of some function which takes at least two parameters and is implemented in all mysql, sqlite and postgesql? Otherwise I'd have to use LOCATE(), which works only on MySQL and that way the whole testcase would be skipped on anything else.

This comment has been minimized.

Copy link
@Majkl578

Majkl578 Dec 9, 2015

Contributor

ROUND() or MOD()?

@meridius meridius force-pushed the meridius:added-whereor branch from 881c210 to 4eda4cf Dec 9, 2015
@meridius meridius force-pushed the meridius:added-whereor branch from 4eda4cf to afc1637 Dec 9, 2015
@dg dg force-pushed the nette:master branch from 6f377ec to 9948322 Feb 8, 2016
@dg dg force-pushed the nette:master branch 4 times, most recently from 0771c2f to 425ad23 Apr 20, 2016
@dg dg force-pushed the nette:master branch from e5a9b9e to 26cc742 May 8, 2016
@dg dg merged commit c6a7061 into nette:master May 22, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dg

This comment has been minimized.

Copy link
Member

dg commented May 22, 2016

Thanks!

dg added a commit that referenced this pull request May 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.