Skip to content

Commit 086ba12

Browse files
committed
Amend WindowFrame docs
The note about WindowFrameBound::Following being only valid "in WindowFrame::end_bound" was both - confusing, as it was based on the ANSI SQL syntax the parser doesn't adhere to -- though it sounded like a promise about the AST one could expect to get from the parser - and incomplete, as the reality is that the bounds validation the SQL engine might want to perform is more complex. For example Postgres documentation says <https://www.postgresql.org/docs/11/sql-expressions.html#SYNTAX-WINDOW-FUNCTIONS>: > Restrictions are that frame_start cannot be UNBOUNDED FOLLOWING, > frame_end cannot be UNBOUNDED PRECEDING, and the frame_end choice > cannot appear earlier in the above list of frame_start and frame_end > options than the frame_start choice does — for example RANGE BETWEEN > CURRENT ROW AND offset PRECEDING is not allowed. But, for example, > ROWS BETWEEN 7 PRECEDING AND 8 PRECEDING is allowed, even though it > would never select any rows.
1 parent f31636d commit 086ba12

File tree

2 files changed

+10
-6
lines changed

2 files changed

+10
-6
lines changed

src/ast/mod.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,11 +292,16 @@ impl fmt::Display for WindowSpec {
292292

293293
/// Specifies the data processed by a window function, e.g.
294294
/// `RANGE UNBOUNDED PRECEDING` or `ROWS BETWEEN 5 PRECEDING AND CURRENT ROW`.
295+
///
296+
/// Note: The parser does not validate the specified bounds; the caller should
297+
/// reject invalid bounds like `ROWS UNBOUNDED FOLLOWING` before execution.
295298
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
296299
pub struct WindowFrame {
297300
pub units: WindowFrameUnits,
298301
pub start_bound: WindowFrameBound,
299-
/// The right bound of the `BETWEEN .. AND` clause.
302+
/// The right bound of the `BETWEEN .. AND` clause. The end bound of `None`
303+
/// indicates the shorthand form (e.g. `ROWS 1 PRECEDING`), which must
304+
/// behave the same as `end_bound = WindowFrameBound::CurrentRow`.
300305
pub end_bound: Option<WindowFrameBound>,
301306
// TBD: EXCLUDE
302307
}
@@ -334,14 +339,14 @@ impl FromStr for WindowFrameUnits {
334339
}
335340
}
336341

342+
/// Specifies [WindowFrame]'s `start_bound` and `end_bound`
337343
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
338344
pub enum WindowFrameBound {
339345
/// `CURRENT ROW`
340346
CurrentRow,
341347
/// `<N> PRECEDING` or `UNBOUNDED PRECEDING`
342348
Preceding(Option<u64>),
343-
/// `<N> FOLLOWING` or `UNBOUNDED FOLLOWING`. This can only appear in
344-
/// [WindowFrame::end_bound].
349+
/// `<N> FOLLOWING` or `UNBOUNDED FOLLOWING`.
345350
Following(Option<u64>),
346351
}
347352

src/parser.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,16 +339,15 @@ impl Parser {
339339
})
340340
}
341341

342-
/// "CURRENT ROW" | ( (<positive number> | "UNBOUNDED") ("PRECEDING" | FOLLOWING) )
342+
/// Parse `CURRENT ROW` or `{ <positive number> | UNBOUNDED } { PRECEDING | FOLLOWING }`
343343
pub fn parse_window_frame_bound(&mut self) -> Result<WindowFrameBound, ParserError> {
344344
if self.parse_keywords(vec!["CURRENT", "ROW"]) {
345345
Ok(WindowFrameBound::CurrentRow)
346346
} else {
347347
let rows = if self.parse_keyword("UNBOUNDED") {
348348
None
349349
} else {
350-
let rows = self.parse_literal_uint()?;
351-
Some(rows)
350+
Some(self.parse_literal_uint()?)
352351
};
353352
if self.parse_keyword("PRECEDING") {
354353
Ok(WindowFrameBound::Preceding(rows))

0 commit comments

Comments
 (0)