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

Add relative date shorthands to Query Builder #54408

Open
wants to merge 1 commit into
base: 11.x
Choose a base branch
from

Conversation

jasonmccreary
Copy link
Contributor

@jasonmccreary jasonmccreary commented Jan 29, 2025

I PR'd a version of this almost 3 years ago and still reach for it. So I figured I'd give it another shot.

This adds a few relative, human readable shorthands for working with dates (cause dates are hard).

$archivedPost = Post::wherePast('archived_at')->get();
$currentReleases = Release::whereToday('released_at')->get();
$activeTokens = Token::whereFuture('expire_at')->get();

While I appreciate this adds weight to this ever growing class, there are dozens of other shorthands. Queries are something devs write everyday. As such, I believe any shorthand continues to improve the DX of Laravel.

All of these methods have their where, or, and orWhereNot counterparts. All methods also accept an array of column names as the first argument. The wherePast and whereFuture have an optional second argument to set $now.

Copy link
Contributor

@shaedrich shaedrich left a comment

Choose a reason for hiding this comment

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

I can't say, I like the naming, though. I'd prefer something like whereDateBefore and whereDateAfter or the like 🤔

src/Illuminate/Database/Query/Builder.php Show resolved Hide resolved
@selcukcukur
Copy link
Contributor

selcukcukur commented Jan 30, 2025

It would be nice if there was a trait class for date relations like DateRelations which would simplify all date related operations with the least possible repetition using regular expressions. However, the pull request you submitted does not seem like a good solution.

@nexxai
Copy link
Contributor

nexxai commented Jan 30, 2025

I am shocked that there is any resistance to this at all. The changes are small, solve a common paper cut, and don't impose any obvious performance penalties. I am definitely all for this.

@shaedrich
Copy link
Contributor

shaedrich commented Jan 30, 2025

It would be nice if there was a trait class for date relations like DateRelations which would simplify all date related operations with the least possible repetition using regular expressions. However, the pull request you submitted does not seem like a good solution.

@selcukcukur There could be a trait for all where combinations, which are quite a lot already.

I am shocked that there is any resistance to this at all. The changes are small, solve a common paper cut, and don't impose any obvious performance penalties. I am definitely all for this.

@nexxai It adds yet another where method, but no, I'm not generally against it

@tpetry
Copy link
Contributor

tpetry commented Jan 30, 2025

For PostgreSQL this would have to use a full iso-8601 date with timezone.

@jasonmccreary
Copy link
Contributor Author

jasonmccreary commented Jan 30, 2025

@tpetry, do you know if the underlying where builder method formats a Carbon object full ISO-8601. My only real concern here is the pre-formatting versus just passing along Carbon::now(). However, the original PR had several comments about using sub-second precision.


Follow-up: A quick dump shows that where('date_column', now()) currently does not format as ISO-8601 with timezone on Postgres. So while you may be right, this PR at least aligns with current date formatting the query builder would do in Postgres (as far as I can tell).

@tpetry
Copy link
Contributor

tpetry commented Jan 30, 2025

Yeah, its not correct currently for PG. I think the best approach would be copying the implementation of substituteBindings so grammar-specific behaviours are accounted for:

if ($value instanceof DateTimeInterface) {
    $bindings[$key] = $value->format($grammar->getDateFormat());
 }

@jasonmccreary
Copy link
Contributor Author

jasonmccreary commented Jan 30, 2025

@tpetry, my humble vote would be to merge this and then address that in a separate PR for where (possibly targetting Laravel 12). These methods could then simply defer to where instead of pre-format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants