Ted-Jiang commented on code in PR #4516:
URL: https://github.com/apache/arrow-datafusion/pull/4516#discussion_r1040387998
##########
datafusion/expr/src/window_frame.rs:
##########
@@ -66,33 +66,51 @@ impl TryFrom<ast::WindowFrame> for WindowFrame {
None => WindowFrameBound::CurrentRow,
};
- if let WindowFrameBound::Following(ScalarValue::Utf8(None)) =
start_bound {
- Err(DataFusionError::Execution(
- "Invalid window frame: start bound cannot be unbounded
following"
- .to_owned(),
- ))
- } else if let WindowFrameBound::Preceding(ScalarValue::Utf8(None)) =
end_bound {
- Err(DataFusionError::Execution(
- "Invalid window frame: end bound cannot be unbounded preceding"
- .to_owned(),
- ))
- } else {
- let units = value.units.into();
- Ok(Self {
- units,
- start_bound,
- end_bound,
- })
- }
+ if let WindowFrameBound::Following(val) = &start_bound {
+ if val.is_null() {
+ return Err(DataFusionError::Execution(
+ "Invalid window frame: start bound cannot be unbounded
following"
+ .to_owned(),
+ ));
+ }
+ } else if let WindowFrameBound::Preceding(val) = &end_bound {
+ if val.is_null() {
+ return Err(DataFusionError::Execution(
+ "Invalid window frame: end bound cannot be unbounded
preceding"
+ .to_owned(),
+ ));
+ }
+ };
+ Ok(Self {
+ units: value.units.into(),
+ start_bound,
+ end_bound,
+ })
}
}
-impl Default for WindowFrame {
- fn default() -> Self {
- WindowFrame {
- units: WindowFrameUnits::Range,
- start_bound: WindowFrameBound::Preceding(ScalarValue::Utf8(None)),
- end_bound: WindowFrameBound::CurrentRow,
+impl WindowFrame {
+ /// Creates a new, default window frame (with the meaning of default
depending on whether the
+ /// frame contains an `ORDER BY` clause.
+ pub fn new(has_order_by: bool) -> Self {
Review Comment:
Thanks for this fix, i also want to change this default behavior.
##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -573,9 +573,7 @@ message WindowExprNode {
repeated LogicalExprNode partition_by = 5;
repeated LogicalExprNode order_by = 6;
// repeated LogicalExprNode filter = 7;
- oneof window_frame {
- WindowFrame frame = 8;
- }
+ WindowFrame window_frame = 8;
Review Comment:
👍
--
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]
