Skip to content
This repository was archived by the owner on Aug 22, 2023. It is now read-only.

PHPORM-60 Fix Query on whereDate, whereDay, whereMonth, whereYear #28

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ All notable changes to this project will be documented in this file.
- Remove `Query\Builder::whereAll($column, $values)`. Use `Query\Builder::where($column, 'all', $values)` instead. [#16](https://github.com/GromNaN/laravel-mongodb-private/pull/16) by [@GromNaN](https://github.com/GromNaN).
- Fix validation of unique values when the validated value is found as part of an existing value. [#21](https://github.com/GromNaN/laravel-mongodb-private/pull/21) by [@GromNaN](https://github.com/GromNaN).
- Support `%` and `_` in `like` expression [#17](https://github.com/GromNaN/laravel-mongodb-private/pull/17) by [@GromNaN](https://github.com/GromNaN).
- Fix Query on `whereDate`, `whereDay`, `whereMonth`, `whereYear`, `whereTime` to use MongoDB operators [#2376](https://github.com/jenssegers/laravel-mongodb/pull/2376) by [@Davpyu](https://github.com/Davpyu)

## [3.9.2] - 2022-09-01

Expand Down
107 changes: 77 additions & 30 deletions src/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Illuminate\Database\Query\Builder as BaseBuilder;
use Illuminate\Database\Query\Expression;
use Illuminate\Support\Arr;
use Illuminate\Support\Carbon;
use Illuminate\Support\Collection;
use Illuminate\Support\LazyCollection;
use Illuminate\Support\Str;
Expand Down Expand Up @@ -116,6 +117,7 @@ class Builder extends BaseBuilder
* @var array
*/
protected $conversion = [
'=' => 'eq',
'!=' => 'ne',
'<>' => 'ne',
'<' => 'lt',
Expand Down Expand Up @@ -1084,7 +1086,7 @@ protected function compileWhereBasic(array $where): array
$operator = $operator === 'regex' ? '=' : 'not';
}

if (! isset($operator) || $operator == '=') {
if (! isset($operator) || $operator === '=' || $operator === 'eq') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could result in a subtle behavioral change when using MongoDB\BSON\Regex objects. Quoting $eq docs:

Specifying the $eq
operator is equivalent to using the form { field: } except when the is a regular expression. See below for examples.

Copy link
Owner Author

Choose a reason for hiding this comment

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

{ field: { eq: Regex } } is not supported ATM as it will always end to { field: Regex } for matching the regex. I don't think storing Regex in database is common enough to fix it know. PHPORM-74

$query = [$column => $value];
} else {
$query = [$column => ['$'.$operator => $value]];
Expand Down Expand Up @@ -1189,12 +1191,41 @@ protected function compileWhereBetween(array $where): array
*/
protected function compileWhereDate(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;
$startOfDay = new UTCDateTime(Carbon::parse($where['value'])->startOfDay());
$endOfDay = new UTCDateTime(Carbon::parse($where['value'])->endOfDay());

return $this->compileWhereBasic($where);
return match($where['operator']) {
'eq', '=' => [
$where['column'] => [
'$gte' => $startOfDay,
'$lte' => $endOfDay,
],
],
'ne' => [
'$or' => [
Copy link
Owner Author

Choose a reason for hiding this comment

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

I tested this alternative, but got segfaults.

'ne' => ['$not' => [
    $column => [
        '$gte' => $startOfDay,
        '$lte' => $endOfDay,
    ],
]],

Copy link
Collaborator

@jmikola jmikola Aug 21, 2023

Choose a reason for hiding this comment

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

got segfaults

Segfaults in mongod (executing the query) or PHP (executing this match construct)?

I'm trying to think about how this would trigger a segfault in PHP. The only thing I can think of is that $column is referenced at multiple levels of the same return value. If so, that seems like something you might be able to isolate in a PHPT test. I'd be happy to dive into it further if so.

If this is actually a mongod segfault, I assume it'd be reproducible in the shell and would warrant a new SERVER ticket.

[
$where['column'] => [
'$lt' => $startOfDay,
],
],
[
$where['column'] => [
'$gt' => $endOfDay,
],
],
],
],
'lt', 'gte' => [
$where['column'] => [
'$'.$where['operator'] => $startOfDay,
],
],
'gt', 'lte' => [
$where['column'] => [
'$'.$where['operator'] => $endOfDay,
],
],
};
}

/**
Expand All @@ -1203,12 +1234,16 @@ protected function compileWhereDate(array $where): array
*/
protected function compileWhereMonth(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;

return $this->compileWhereBasic($where);
return [
'$expr' => [
'$'.$where['operator'] => [
[
'$month' => '$'.$where['column'],
],
(int) $where['value'],
],
],
];
}

/**
Expand All @@ -1217,12 +1252,16 @@ protected function compileWhereMonth(array $where): array
*/
protected function compileWhereDay(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;

return $this->compileWhereBasic($where);
return [
'$expr' => [
'$'.$where['operator'] => [
[
'$dayOfMonth' => '$'.$where['column'],
],
(int) $where['value'],
],
],
];
}

/**
Expand All @@ -1231,12 +1270,16 @@ protected function compileWhereDay(array $where): array
*/
protected function compileWhereYear(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;

return $this->compileWhereBasic($where);
return [
'$expr' => [
'$'.$where['operator'] => [
[
'$year' => '$'.$where['column'],
],
(int) $where['value'],
],
],
];
}

/**
Expand All @@ -1245,12 +1288,16 @@ protected function compileWhereYear(array $where): array
*/
protected function compileWhereTime(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;

return $this->compileWhereBasic($where);
return [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noted the original author's comment in mongodb/laravel-mongodb#2376:

whereTime isnt possible to fix rn since i cant find logic to query like time() on sql

Does this address that concern?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, fixed. There is no dedicated operator, but this was possible using $dateToString.

'$expr' => [
'$'.$where['operator'] => [
[
'$dateToString' => ['date' => '$'.$where['column'], 'format' => '%H:%M:%S'],
],
$where['value'],
],
],
];
}

/**
Expand Down
9 changes: 5 additions & 4 deletions tests/Models/Birthday.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@
*
* @property string $name
* @property string $birthday
* @property string $day
* @property string $month
* @property string $year
* @property string $time
*/
class Birthday extends Eloquent
{
protected $connection = 'mongodb';
protected $collection = 'birthday';
protected $fillable = ['name', 'birthday', 'day', 'month', 'year', 'time'];
protected $fillable = ['name', 'birthday'];

protected $casts = [
'birthday' => 'datetime',
];
}
132 changes: 132 additions & 0 deletions tests/Query/BuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,138 @@ function (Builder $builder) {
fn (Builder $builder) => $builder->where('name', 'not regex', '/^acme$/si'),
];

yield 'where date' => [
['find' => [['created_at' => [
'$gte' => new UTCDateTime(new DateTimeImmutable('2018-09-30 00:00:00.000 +00:00')),
'$lte' => new UTCDateTime(new DateTimeImmutable('2018-09-30 23:59:59.999 +00:00')),
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '2018-09-30'),
];

yield 'where date DateTimeImmutable' => [
['find' => [['created_at' => [
'$gte' => new UTCDateTime(new DateTimeImmutable('2018-09-30 00:00:00.000 +00:00')),
'$lte' => new UTCDateTime(new DateTimeImmutable('2018-09-30 23:59:59.999 +00:00')),
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '=', new DateTimeImmutable('2018-09-30 15:00:00 +02:00')),
];

yield 'where date !=' => [
['find' => [['$or' => [
['created_at' => ['$lt' => new UTCDateTime(new DateTimeImmutable('2018-09-30 00:00:00.000 +00:00'))]],
['created_at' => ['$gt' => new UTCDateTime(new DateTimeImmutable('2018-09-30 23:59:59.999 +00:00'))]],
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '!=', '2018-09-30'),
];

yield 'where date <' => [
['find' => [['created_at' => [
'$lt' => new UTCDateTime(new DateTimeImmutable('2018-09-30 00:00:00.000 +00:00')),
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '<', '2018-09-30'),
];

yield 'where date >=' => [
['find' => [['created_at' => [
'$gte' => new UTCDateTime(new DateTimeImmutable('2018-09-30 00:00:00.000 +00:00')),
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '>=', '2018-09-30'),
];

yield 'where date >' => [
['find' => [['created_at' => [
'$gt' => new UTCDateTime(new DateTimeImmutable('2018-09-30 23:59:59.999 +00:00')),
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '>', '2018-09-30'),
];

yield 'where date <=' => [
['find' => [['created_at' => [
'$lte' => new UTCDateTime(new DateTimeImmutable('2018-09-30 23:59:59.999 +00:00')),
]], []]],
fn (Builder $builder) => $builder->whereDate('created_at', '<=', '2018-09-30'),
];

yield 'where day' => [
['find' => [['$expr' => [
'$eq' => [
['$dayOfMonth' => '$created_at'],
5,
],
]], []]],
fn (Builder $builder) => $builder->whereDay('created_at', 5),
];

yield 'where day > string' => [
['find' => [['$expr' => [
'$gt' => [
['$dayOfMonth' => '$created_at'],
5,
],
]], []]],
fn (Builder $builder) => $builder->whereDay('created_at', '>', '05'),
];

yield 'where month' => [
['find' => [['$expr' => [
'$eq' => [
['$month' => '$created_at'],
10,
],
]], []]],
fn (Builder $builder) => $builder->whereMonth('created_at', 10),
];

yield 'where month > string' => [
['find' => [['$expr' => [
'$gt' => [
['$month' => '$created_at'],
5,
],
]], []]],
fn (Builder $builder) => $builder->whereMonth('created_at', '>', '05'),
];

yield 'where year' => [
['find' => [['$expr' => [
'$eq' => [
['$year' => '$created_at'],
2023,
],
]], []]],
fn (Builder $builder) => $builder->whereYear('created_at', 2023),
];

yield 'where year > string' => [
['find' => [['$expr' => [
'$gt' => [
['$year' => '$created_at'],
2023,
],
]], []]],
fn (Builder $builder) => $builder->whereYear('created_at', '>', '2023'),
];

yield 'where time' => [
['find' => [['$expr' => [
'$eq' => [
['$dateToString' => ['date' => '$created_at', 'format' => '%H:%M:%S']],
'10:11:12',
],
]], []]],
fn (Builder $builder) => $builder->whereTime('created_at', '10:11:12'),
];

yield 'where time >' => [
['find' => [['$expr' => [
'$gt' => [
['$dateToString' => ['date' => '$created_at', 'format' => '%H:%M:%S']],
'10:11:12',
],
]], []]],
fn (Builder $builder) => $builder->whereTime('created_at', '>', '10:11:12'),
];

/** @see DatabaseQueryBuilderTest::testBasicSelectDistinct */
yield 'distinct' => [
['distinct' => ['foo', [], []]],
Expand Down
Loading