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

GromNaN · 2023-08-21T13:34:20Z

Created from mongodb/laravel-mongodb#2376

So basically, current whereDate, whereDay, whereMonth and whereYear is query on different column with specific need (like whereDate you need column that has value YYYY-MM-DD etc) using basic comparison.

As we all know that isnt how whereDate works on sql, this PR will generate query using built in $dayOfMonth, $month, $year on whereDay, whereMonth, and whereYear, while whereDate basically generate range date(since mongodb doesnt have date() equivalent like on sql).

whereDate uses $lt & $gt comparison with UTCDateTime objects
whereDay uses $dayOfWeek
whereMonth uses $month
whereYear uses $year
whereTime uses $dateToString to format into H:M:S

…ective query rather than using basic comparison

GromNaN · 2023-08-21T16:34:23Z

tests/QueryTest.php

+        Birthday::create(['name' => 'Robert Doe', 'birthday' => new DateTimeImmutable('2021-05-12 10:53:14'), 'time' => '10:53:14']);
+        Birthday::create(['name' => 'Mark Moe', 'birthday' => new DateTimeImmutable('2021-05-12 10:53:15'), 'time' => '10:53:15']);
+        Birthday::create(['name' => 'Mark Moe', 'birthday' => new DateTimeImmutable('2022-05-12 10:53:16'), 'time' => '10:53:16']);
+        Birthday::create(['name' => 'Boo']);


Added a document without birthday field to test the behavior.

GromNaN · 2023-08-21T16:35:25Z

src/Query/Builder.php

+                ],
+            ],
+            'ne' => [
+                '$or' => [


I tested this alternative, but got segfaults.

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

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.

jmikola · 2023-08-21T21:06:37Z

src/Query/Builder.php

@@ -1084,7 +1086,7 @@ protected function compileWhereBasic(array $where): array
            $operator = $operator === 'regex' ? '=' : 'not';
        }

-        if (! isset($operator) || $operator == '=') {
+        if (! isset($operator) || $operator === '=' || $operator === 'eq') {


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.

{ 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

jmikola · 2023-08-21T21:10:19Z

src/Query/Builder.php

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

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


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.

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

jmikola · 2023-08-21T21:12:39Z

src/Query/Builder.php

+            '$expr' => [
+                '$'.$operator => [
+                    [
+                        '$month' => '$'.$column,


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?

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

jmikola · 2023-08-21T21:16:19Z

src/Query/Builder.php

-        $where['value'] = $value;
-
-        return $this->compileWhereBasic($where);
+        return [


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?

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

tests/Models/Birthday.php

GromNaN · 2023-08-22T10:28:44Z

Moved to mongodb/laravel-mongodb#2572

GromNaN requested a review from alcaeus August 21, 2023 13:34

GromNaN marked this pull request as draft August 21, 2023 14:00

Davpyu and others added 2 commits August 21, 2023 16:08

Fix whereDate, whereMonth, whereYear, whereTime to use $expr and resp…

4ec7a5f

…ective query rather than using basic comparison

Add and fix tests

40ee389

GromNaN force-pushed the pr/2376 branch from e896c15 to 40ee389 Compare August 21, 2023 16:01

Fix whereDate

9ffc492

GromNaN marked this pull request as ready for review August 21, 2023 16:31

GromNaN requested a review from jmikola August 21, 2023 16:31

GromNaN commented Aug 21, 2023

View reviewed changes

PHPORM-60 Native support for whereTime

4cc05b3

GromNaN force-pushed the pr/2376 branch from 39b62de to 4cc05b3 Compare August 21, 2023 19:51

jmikola reviewed Aug 21, 2023

View reviewed changes

Remove magic extract to use explicit array access to options

5695e38

GromNaN commented Aug 21, 2023

View reviewed changes

tests/Models/Birthday.php Outdated Show resolved Hide resolved

Update tests/Models/Birthday.php

0e018b1

GromNaN closed this Aug 22, 2023

jmikola mentioned this pull request Aug 22, 2023

PHPORM-60 Fix Query on whereDate, whereDay, whereMonth, whereYear mongodb/laravel-mongodb#2572

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

GromNaN commented Aug 21, 2023 •

edited

Loading

Uh oh!

GromNaN Aug 21, 2023 •

edited

Loading

Uh oh!

GromNaN Aug 21, 2023

Uh oh!

jmikola Aug 21, 2023 •

edited

Loading

Uh oh!

jmikola Aug 21, 2023

Uh oh!

GromNaN Aug 21, 2023

Uh oh!

jmikola Aug 21, 2023

Uh oh!

GromNaN Aug 21, 2023

Uh oh!

jmikola Aug 21, 2023

Uh oh!

GromNaN Aug 21, 2023

Uh oh!

jmikola Aug 21, 2023

Uh oh!

GromNaN Aug 21, 2023

Uh oh!

Uh oh!

GromNaN commented Aug 22, 2023

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

GromNaN commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GromNaN Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmikola Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GromNaN commented Aug 22, 2023

Uh oh!

Uh oh!

GromNaN commented Aug 21, 2023 •

edited

Loading

GromNaN Aug 21, 2023 •

edited

Loading

jmikola Aug 21, 2023 •

edited

Loading