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

Fix Query on whereDate, whereDay, whereMonth, whereYear #2376

Closed
wants to merge 14 commits into from

Conversation

Davpyu
Copy link
Contributor

@Davpyu Davpyu commented Mar 28, 2022

Fix issues #2334

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).

Example:
Collection on mongodb

db.birthdays.insert([
    { '_id': 1, 'name': 'John Doe', 'birthday_at': new Date('2002-02-01') },
    { '_id': 2, 'name': 'Nash Doe', 'birthday_at': new Date('1992-06-20') },
    { '_id': 3, 'name': 'Jane Doe', 'birthday_at': new Date('2000-01-15') },
]);

Eloquent if this merged

Birthday::whereDate('birthday_at', '2002-02-01')->get();
// generated query: birthdays.find({ 'birthday_at' => { '$gte' => '2002-02-01T00:00:00.000Z', '$lte' => '2002-02-01T23:59:59.999Z' } })

Birthday::whereDay('birthday_at', 15)->get();
// generated query: birthdays.find({ '$expr' => { '$eq' => [{ '$dayOfMonth' => 'birthday_at' }, 15] } })

Birthday::whereMonth('birthday_at', 6)->get();
// generated query: birthdays.find({ '$expr' => { '$eq' => [{ '$month' => 'birthday_at' }, 6] } })

Birthday::whereYear('birthday_at', 2002)->get();
// generated query: birthdays.find({ '$expr' => { '$eq' => [{ '$year' => 'birthday_at' }, 2002] } })

Note:

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

gorkie
gorkie previously approved these changes Apr 4, 2022
charlieuki
charlieuki previously approved these changes Apr 26, 2022
@Davpyu
Copy link
Contributor Author

Davpyu commented Jul 13, 2022

Any update for this one?

@divine
Copy link
Contributor

divine commented Jul 13, 2022

Any update for this one?

Hello,

Please remove styleci changes as this isn't related to this PR.

I don't see any check runs for this PR?

Thanks!

$this->assertCount(5, $month);
}

public function testWhereYear(): void
{
$year = Birthday::whereYear('year', '2021')->get();
$year = Birthday::whereYear('birthday', 2021)->get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't modify current tests to see if it's breaking current applications.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as for this, it is breaking change.
Why?
its from comparing between string (current whereYear etc) to comparing datetime (on mongodb) with string/int
So what do i need to do?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello,

Just don't touch current tests and add additional to see if it changes in behavior.

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@Davpyu can you revert old code and additional test cases ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, i've reverted this. Please let me know if there are any commit that need to be reverted

@Davpyu
Copy link
Contributor Author

Davpyu commented Jul 28, 2022

Okay i'll update after working hours

@divine
Copy link
Contributor

divine commented Nov 4, 2022

Hello @Davpyu,

Can you please update or I should take care of it?

Thanks!

@Davpyu
Copy link
Contributor Author

Davpyu commented Nov 16, 2022

Hello @Davpyu,

Can you please update or I should take care of it?

Thanks!

im sorry for not responding so long because busy with work, yes you can take care of this PR if you want it

@Davpyu Davpyu dismissed stale reviews from charlieuki and gorkie via f824ff9 January 18, 2023 09:24
@Davpyu
Copy link
Contributor Author

Davpyu commented Jan 18, 2023

Any update for this one?

Hello,

Please remove styleci changes as this isn't related to this PR.

I don't see any check runs for this PR?

Thanks!

Hello, i've reverted the styleci for this PR.
Thanks

@GromNaN
Copy link
Member

GromNaN commented Aug 22, 2023

Hello @Davpyu, thanks for your work on this feature. I'm taking over your PR in #2572 to get it merged in the next release.

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

I found a solution using $dateToString.

@alcaeus
Copy link
Member

alcaeus commented Aug 22, 2023

Closing in favour of #2572.

@alcaeus alcaeus closed this Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants