[fix](functions)Preserve TIMESTAMPTZ in LEAD/LAG by adding TimeStampTzType signature matching#62779
Conversation
…zType signature matching
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
No blocking issues found in this PR.
Checkpoint review:
- Goal / correctness: The change fixes Nereids
LEAD/LAGsignature selection forTIMESTAMPTZby addingTimeStampTzType.WILDCARDtoRequireTrivialTypes, soLead/Lagno longer need to fall back to aDATETIMEV2signature on this path. The added unit test covers signature resolution for both functions. - Scope / minimality: The patch is small and focused: one signature-list update plus a targeted FE test.
- Concurrency: Not involved; this is static FE type-resolution code with no new shared mutable state or locking.
- Lifecycle / initialization: No special lifecycle concerns beyond the existing static signature initialization in
Lead/Lag; the new type follows the same pattern. - Config / compatibility: No new config, protocol, persistence, or storage compatibility impact.
- Parallel code paths: Both
LEADandLAGare updated consistently. I did not find another FE signature table for these functions outside the Nereids builtin registry. - Test coverage: The new FE unit test checks
TIMESTAMPTZsignature matching for bothLEADandLAG. I could not run it locally in this runner becausethirdparty/installed/bin/protocis missing, so runtime verification is blocked by environment setup rather than by the patch itself. - Observability / transactions / writes / FE-BE variables: Not applicable here.
- Performance: No concern; this only adds one extra candidate type to a tiny static signature list.
- Other issues: No blocking findings. Minor note only: the comment in
RequireTrivialTypesstill says the list is equal toType.trivialTypes, butType.trivialTypesdoes not currently includeTIMESTAMPTZ.
User focus points:
- No additional user-provided review focus was supplied.
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
No blocking issues found in this PR.\n\nCheckpoint review:\n- Goal / correctness: The change fixes Nereids LEAD/LAG signature selection for TIMESTAMPTZ by adding TimeStampTzType.WILDCARD to RequireTrivialTypes, so these functions can preserve TIMESTAMPTZ instead of falling back to DATETIMEV2 on this path. The added unit test covers signature resolution for both functions.\n- Scope / minimality: The patch is small and focused: one signature-list update plus a targeted FE unit test.\n- Concurrency: Not involved; this is FE type-resolution/signature code with no new shared mutable runtime state or locking.\n- Lifecycle / initialization: No special lifecycle concerns beyond existing static signature initialization in Lead/Lag; the new wildcard follows the existing DateTimeV2/TimeV2 pattern.\n- Config / compatibility: No new config, protocol, persistence, storage format, or rolling-upgrade compatibility impact.\n- Parallel code paths: LEAD and LAG are updated consistently. Related window functions using retArgType/AnyDataType already preserve argument type through a different path.\n- Tests: The new FE unit test checks TIMESTAMPTZ matching for both LEAD and LAG. I attempted to run ./run-fe-ut.sh --run org.apache.doris.nereids.trees.expressions.functions.window.TimestampTzLeadLagSignatureTest locally, but generated-source.sh failed because thirdparty/installed/bin/protoc is missing in this runner checkout; remote PR status shows FE UT success.\n- Observability / transactions / writes / FE-BE variables: Not applicable.\n- Performance: No concern; this adds one candidate type to a small static signature list.\n- Other issues: No blocking findings. Minor non-blocking note: the RequireTrivialTypes comment says the list is equal to Type.trivialTypes, but Type.trivialTypes does not currently include TIMESTAMPTZ.\n\nUser focus points:\n- No additional user-provided review focus was supplied.
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
…zType signature matching (#62779) Problem: When LEAD/LAG use ExplicitlyCastableSignature, Nereids could fail to find a TIMESTAMPTZ signature and fall back to coercion to DateTimeV2Type, causing an injected cast like cast(ts_tz as DATETIMEV2(6)) and losing the timezone suffix.
…zType signature matching (#62779) Problem: When LEAD/LAG use ExplicitlyCastableSignature, Nereids could fail to find a TIMESTAMPTZ signature and fall back to coercion to DateTimeV2Type, causing an injected cast like cast(ts_tz as DATETIMEV2(6)) and losing the timezone suffix.
Problem: When LEAD/LAG use ExplicitlyCastableSignature, Nereids could fail to find a TIMESTAMPTZ signature and fall back to coercion to DateTimeV2Type, causing an injected cast like cast(ts_tz as DATETIMEV2(6)) and losing the timezone suffix.
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)