-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add m
alias for minute
duration literal
#136448
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
Conversation
Pinging @elastic/es-analytical-engine (Team:Analytics) |
🔍 Preview links for changed docs |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
Hi @felixbarny, I've created a changelog YAML for you. |
Note that the However, we can think about de-emphasizing or deprecating |
See also this note from the date_diff docs:
Seems like it's fine that the date_diff and time span/duration units differ. |
LGTM, adding @bpintea since he's been working in this area and for awareness of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for a comment, it LGTM.
m
isn't quite unambiguous, unfortunately. Worse off, we have a DATE_DIFF function that uses its own set of units and uses m
for month, inline with other SQL implementations of the similar function, like MS SQL. (And the conflict/confusion shows already, like in #136426, despite our docs clarification). But I guess we can assume that minute
has a higher usage generally and prioritise it.
Allows ES|QL queries like this:
There's a potential ambiguity with
month
butm
is commonly used in other query languages to mean minutes so I think that's essentially common practice.Today, we don't support
m
and a query would fail withUnexpected temporal unit: 'm'
. So this change is not breaking existing behavior.Closes #135552