POSSIBLEwbwb commented on code in PR #66860:
URL: https://github.com/apache/doris/pull/66860#discussion_r3809507097
##########
pytest/qe/palo2/src/test_query_datetime_function.py:
##########
@@ -1007,6 +1007,37 @@ def test_query_time_convert_tz():
runner.check2(line1, line2)
+def test_query_timezone_hour_minute():
+ """
+ {
+ "title": "test_query_datetime_function.test_query_timezone_hour_minute",
+ "describe": "test for timezone_hour and timezone_minute",
+ "tag": "function,p0"
+ }
+ """
+ # UTC+08:00 has no DST, the offset of the session timezone is the same
+ # for every instant, so timezone_hour always returns 8 here.
+ runner.init("set time_zone = '+08:00'")
Review Comment:
Thanks for the thorough review! All three blocking findings are addressed.
The branch now contains:
**Commit `aee93c9b`** — the three fixes:
**[P1] Input-zone semantics** — I resolved the contract by making the
session-zone semantics explicit (your "explicitly scope" option): a
`TIMESTAMPTZ` value in Doris stores only the UTC instant — the input zone is
discarded at parse time (`TimestampTzValue` is 8 bytes of UTC microseconds,
`be/src/core/value/timestamptz_value.h`), so recovering the input zone requires
redesigning the storage format, well beyond this function pair. The scoping is
now explicit in:
- FE javadoc on `TimezoneHour` / `TimezoneMinute` (session-zone extraction,
divergence from Trino noted).
- BE comment above the offset extraction in
`function_timezone_hour_minute.cpp`.
- Documentation (en/zh) with a divergence example: input `-04:30`, session
`+08:00` → returns 8/0; Trino would return -4/-30.
- End-to-end pytest case with differing input/session zones, and BE unit
tests `const_input` + `session_zone_wins_over_input_zone`.
**[P1] Session-timezone dependency (determinism)** — modeled on both engines:
- FE: `isDeterministic()` overridden to `false` in
`TimezoneHour`/`TimezoneMinute`, which sets
`StatementContext.hasNondeterministic` during analysis (same mechanism as
`now()`/`current_date()`): the statement is excluded from SQL cache, and the
expression is not folded into reusable prepared plans or admitted into MV
expressions.
- BE: `"timezone_hour"`/`"timezone_minute"` added to
`NON_DETERMINISTIC_FUNCTIONS` in `vectorized_fn_call.cpp`, so BE-side constant
folding skips them too.
**[P2] pytest connection** — `test_query_timezone_hour_minute` rewritten to
use `do_set_properties_sql`, so each `SET time_zone` and its `SELECT` run on
one connection (this is the established pattern, e.g.
`test_query_union_join.py`). Cases: fixed offset (+08:00), America/New_York
winter/summer (DST), fractional (Asia/Kolkata 5/30), input-zone-vs-session-zone
divergence (`-04:30` input, `+08:00` session), and NULL input.
**Commit `77bbf781`** — robustness: the BE executor now unwraps nullable
before const so the column reaches the plain `ColumnTimeStampTz` data
regardless of wrapper order. (Const columns are only legal at the top level of
a column tree — Doris enforces this — so the reachable combinations are plain /
top-level const / top-level nullable, all covered by the unit tests.)
Verification (local):
- BE unit tests: 6 `FunctionTimezoneHourMinuteTest` cases pass against the
ASAN UT build (fixed, DST, fractional, const input, session-vs-input zone,
nullable).
- FE compiles cleanly (fe-core, JDK 17 / Maven 3.9).
- The pytest case was run against a local single-node cluster earlier in
this PR's development.
Honest caveats:
- I did not add prepared-query / MV regression tests: the palo2 pytest suite
has no prepared/MV infrastructure, and I have no local regression-framework
environment to validate a new `.groovy` suite (I prefer not to submit
unvalidated tests). The determinism fix uses the same standard mechanism as
existing non-deterministic builtins; if you'd like, I can follow up with a
`prepared_stmt_p0` / `mv_p0` groovy case once I can run the regression
framework locally.
- The matching documentation update is in apache/doris-website#4070 (pushed;
not linked in the PR body since I have no token to comment from here).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
