-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ESQL: Timezone support in DATE_TRUNC, BUCKET and TBUCKET #137450
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
base: main
Are you sure you want to change the base?
Conversation
… ones that use it
# Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormatTests.java
# Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java
# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/TBucket.java
|
Hi @ivancea, I've created a changelog YAML for you. |
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.
I made this class for testing purposes, as working with random configurations "with just a single fixed field" was a bit painful. I'm listening for opinions here!
| 1 | 1991-01-01T00:00:00.000Z | ||
| ; | ||
|
|
||
| dateTruncTimezonesTimeDuration |
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.
The new tests begin here!
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.
All date trunc tests were moved to date_trunc.csv-spec, for organization. There was no change or test removed.
I added a comment where the new tests begin in date_trunc.csv-spec
| /** | ||
| * Support timezones in DATE_TRUNC and dependent functions. | ||
| */ | ||
| DATE_TRUNC_TIMEZONE_SUPPORT(Build.current().isSnapshot()), |
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.
The time_zone SET setting is snapshot-only, so this has to be too
| public class Configuration implements Writeable { | ||
|
|
||
| public static final int QUERY_COMPRESS_THRESHOLD_CHARS = KB.toIntBytes(5); | ||
| public static final ZoneId DEFAULT_TZ = ZoneOffset.UTC; |
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's no default anymore here. Now the default is in the QuerySettings setting
Timezone settings (Both the request parameter and the SET setting) are snapshot-only.
Add timezone support to DATE_TRUNC, BUCKET and TBUCKET.
The current timezone is already in the Configuration. This PR:
For reviewers
Please take a good look at the expected results of DATE_TRUNC in
DateTruncTests, and comment any interesting or edge case you can think of!