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

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ec7a5f
Fix whereDate, whereMonth, whereYear, whereTime to use $expr and resp…
Davpyu Mar 27, 2022
40ee389
Add and fix tests
GromNaN Aug 21, 2023
9ffc492
Fix whereDate
GromNaN Aug 21, 2023
4cc05b3
PHPORM-60 Native support for whereTime
GromNaN Aug 21, 2023
5695e38
Remove magic extract to use explicit array access to options
GromNaN Aug 21, 2023
0e018b1
Update tests/Models/Birthday.php
GromNaN Aug 21, 2023
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
View file
Open in desktop
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
97 changes: 80 additions & 17 deletions src/Query/Builder.php
View file
Open in desktop
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 @@ -1191,10 +1193,41 @@ protected function compileWhereDate(array $where): array
{
extract($where);

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

return $this->compileWhereBasic($where);
return match($operator) {
'eq', '=' => [
$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 •
edited
Loading

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.

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

/**
Expand All @@ -1205,10 +1238,18 @@ protected function compileWhereMonth(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;
$value = (int) ltrim($value, '0');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is ltrim() necessary to avoid interpreting a leading zero as octal notation? I would only expect that to apply to integer literals.

In my local testing with a PHP 8.2 REPL, the integer cast seems to ignore leading zeroes.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Exactly, I blindly adopted the PR code without even testing it. Fixed.


return $this->compileWhereBasic($where);
return [
'$expr' => [
'$'.$operator => [
[
'$month' => '$'.$column,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume $column is created by extract($where). Is there any enforcement on that? It seems hard to track given we have no typing/shape information for the $where array.

Is it possible for $column to be a field path (e.g. foo.bar), or would it only be a top-level field name?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Removing all this extract and replacing with array access would be more verbose but a lot more explicit.

],
$value,
],
],
];
}

/**
Expand All @@ -1219,10 +1260,18 @@ protected function compileWhereDay(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;
$value = (int) ltrim($value, '0');

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

/**
Expand All @@ -1233,10 +1282,18 @@ protected function compileWhereYear(array $where): array
{
extract($where);

$where['operator'] = $operator;
$where['value'] = $value;
$value = (int) $value;

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

/**
Expand All @@ -1247,10 +1304,16 @@ 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' => [
'$'.$operator => [
[
'$dateToString' => ['date' => '$'.$column, 'format' => '%H:%M:%S'],
],
$value,
],
],
];
}

/**
Expand Down
9 changes: 5 additions & 4 deletions tests/Models/Birthday.php
View file
Open in desktop
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', 'time'];

protected $casts = [
'birthday' => 'datetime',
];
}
132 changes: 132 additions & 0 deletions tests/Query/BuilderTest.php
View file
Open in desktop
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