Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15115,7 +15115,8 @@ impl<'a> Parser<'a> {

let mut top_before_distinct = false;
let mut top = None;
if self.dialect.supports_top_before_distinct() && self.parse_keyword(Keyword::TOP) {
if self.dialect.supports_top_before_distinct() && self.peek_top_clause() {
self.expect_keyword(Keyword::TOP)?;
top = Some(self.parse_top()?);
top_before_distinct = true;
}
Expand All @@ -15126,7 +15127,8 @@ impl<'a> Parser<'a> {
self.parse_all_or_distinct()?
};

if !self.dialect.supports_top_before_distinct() && self.parse_keyword(Keyword::TOP) {
if !self.dialect.supports_top_before_distinct() && self.peek_top_clause() {
self.expect_keyword(Keyword::TOP)?;
top = Some(self.parse_top()?);
Comment on lines +15131 to 15132

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking one improvement we could do while we're making this change, can we move the expect_keyword into the parse_top function? that way the function is standalone (we can change the other callsite below to remove its expect_keyword to match)

}

Expand Down Expand Up @@ -19386,6 +19388,18 @@ impl<'a> Parser<'a> {
Ok(InterpolateExpr { column, expr })
}

/// Returns true if the parser is positioned at a `TOP` clause, i.e. the
/// `TOP` keyword followed by `(` or a number. When `TOP` is followed by
/// anything else it is an ordinary identifier (column, alias, or table
/// name), not the row-limit clause parsed by [`Self::parse_top`].
fn peek_top_clause(&self) -> bool {
self.peek_keyword(Keyword::TOP)
&& matches!(
self.peek_nth_token_ref(1).token,
Token::LParen | Token::Number(_, _)
)
}
Comment on lines +19395 to +19401

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually instead of having a peek and a parse for the top clause, we could change parse_top_clause to maybe_parse_top_clause() -> Option<>then the callers only need to check the dialect support and call if set


/// Parse a TOP clause, MSSQL equivalent of LIMIT,
/// that follows after `SELECT [DISTINCT]`.
pub fn parse_top(&mut self) -> Result<Top, ParserError> {
Expand Down
17 changes: 17 additions & 0 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15376,6 +15376,23 @@ fn test_select_top() {
dialects.verified_stmt("SELECT TOP 3 DISTINCT a, b, c FROM tbl");
}

#[test]
fn parse_top_as_identifier() {
// `TOP` is only the row-limit clause when followed by `(` or a number.
// Otherwise it is an ordinary identifier and must remain usable as a
// column name, alias, or table name. Covers both the `TOP`-before-distinct
// dialects and the general case.
let dialects = all_dialects_where(|d| d.supports_top_before_distinct());
dialects.verified_stmt("SELECT top FROM tbl");
dialects.verified_stmt("SELECT top, bottom FROM tbl");
dialects.verified_stmt("SELECT top.val FROM tbl AS top");

let dialects = all_dialects_where(|d| !d.supports_top_before_distinct());
dialects.verified_stmt("SELECT top FROM tbl");
dialects.verified_stmt("SELECT top, bottom FROM tbl");
dialects.verified_stmt("SELECT top.val FROM tbl AS top");
}

#[test]
fn parse_bang_not() {
let dialects = all_dialects_where(|d| d.supports_bang_not_operator());
Expand Down