-
Notifications
You must be signed in to change notification settings - Fork 590
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
refactor(optimizer): unify watermark derivation and monotonicity derivation #17554
Conversation
ExprType::AtTimeZone => match self.visit_binary_op(func_call.inputs()) { | ||
(Constant, Constant) => Constant, | ||
(any, Constant) => { | ||
let time_zone = func_call.inputs()[1] | ||
.as_literal() | ||
.and_then(|literal| literal.get_data().as_ref()) | ||
.map(|tz| tz.as_utf8().as_ref()); | ||
// 1. For at_time_zone(timestamp, const timezone) -> timestamptz, when timestamp has some monotonicity, | ||
// the result should have the same monotonicity. | ||
// 2. For at_time_zone(timestamptz, const timezone) -> timestamp, when timestamptz has some monotonicity, | ||
// the result only have the same monotonicity when the timezone is without DST (Daylight Saving Time). | ||
if (func_call.inputs()[0].return_type() == DataType::Timestamp | ||
&& func_call.return_type() == DataType::Timestamptz) | ||
|| time_zone_is_without_dst(time_zone) | ||
{ | ||
any | ||
} else { | ||
Unknown | ||
} | ||
} | ||
_ => Unknown, | ||
}, | ||
ExprType::DateTrunc => match func_call.inputs().len() { | ||
2 => match self.visit_binary_op(func_call.inputs()) { | ||
(Constant, any) => any, | ||
_ => Unknown, | ||
}, | ||
3 => match self.visit_ternary_op(func_call.inputs()) { | ||
(Constant, Constant, Constant) => Constant, | ||
(Constant, any, Constant) => { | ||
let time_zone = func_call.inputs()[2] | ||
.as_literal() | ||
.and_then(|literal| literal.get_data().as_ref()) | ||
.map(|tz| tz.as_utf8().as_ref()); | ||
if time_zone_is_without_dst(time_zone) { | ||
any | ||
} else { | ||
Unknown | ||
} | ||
} | ||
_ => Unknown, | ||
}, | ||
_ => unreachable!(), | ||
}, | ||
ExprType::AddWithTimeZone | ExprType::SubtractWithTimeZone => { | ||
// Requires time zone and interval to be literal, at least for now. | ||
let time_zone = match &func_call.inputs()[2] { | ||
ExprImpl::Literal(lit) => { | ||
lit.get_data().as_ref().map(|tz| tz.as_utf8().as_ref()) | ||
} | ||
_ => return Unknown, | ||
}; | ||
let interval = match &func_call.inputs()[1] { | ||
ExprImpl::Literal(lit) => lit | ||
.get_data() | ||
.as_ref() | ||
.map(|interval| interval.as_interval()), | ||
_ => return Unknown, | ||
}; | ||
let quantitative_only = interval.map_or( | ||
true, // null interval is treated as `interval '1' second` | ||
|v| v.months() == 0 && (v.days() == 0 || time_zone_is_without_dst(time_zone)), | ||
); | ||
match (self.visit_expr(&func_call.inputs()[0]), quantitative_only) { |
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.
Small fixes related to these datetime functions are included when adapting the old WatermarkAnalyzer
code to the new MonotonicityAnalyzer
. PTAL cc @xiangjinwu
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.
general LGTM
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
9f264c4
to
6b9feea
Compare
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.
LGTM
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Previously we have
Nondecreasing
variant ofWatermarkDerivation
to support watermark onproctime
column;When I worked on the continuation of the "monotonicity" property, I realized that we can unify the watermark and monotonicity derivation process, by
WatermarkDerivation
in the same recursive manner as we do forWatermarkDerivation
;WatermarkDerivation
.Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.