Skip to content

Support parsing empty map literal syntax for DuckDB and Genric #1361

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 4, 2024
Merged

Support parsing empty map literal syntax for DuckDB and Genric #1361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
21cdef3
support parsing empty map literal syntax
goldmedal Aug 2, 2024
0763f52
extract parse_comma_separated0
goldmedal Aug 3, 2024
5d81654
use parse_comma_separated0 for parsing array and map elements
goldmedal Aug 3, 2024
4479ecd
fix the doc
goldmedal Aug 3, 2024
239ef53
roll back trailing_commas
goldmedal Aug 4, 2024
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
14 changes: 8 additions & 6 deletions src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2352,12 +2352,14 @@ impl<'a> Parser<'a> {
/// [map]: https://duckdb.org/docs/sql/data_types/map.html#creating-maps
fn parse_duckdb_map_literal(&mut self) -> Result<Expr, ParserError> {
self.expect_token(&Token::LBrace)?;

let fields = self.parse_comma_separated(Self::parse_duckdb_map_field)?;

self.expect_token(&Token::RBrace)?;

Ok(Expr::Map(Map { entries: fields }))
if self.peek_token().token == Token::RBrace {
Copy link
Contributor

@iffyio iffyio Aug 3, 2024

Choose a reason for hiding this comment

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

is it possible to switch to let fields = self.parse_comma_separated0(Self::parse_duckdb_map_field)?; instead? if so that would let us skip the if/else here

Copy link
Contributor Author

@goldmedal goldmedal Aug 3, 2024

Choose a reason for hiding this comment

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

I don't think we can do it. It seems that parse_comma_separated0 is for parentheses, but MAP uses braces.
https://github.com/sqlparser-rs/sqlparser-rs/blob/d49acc67b13e1d68f2e6a25546161a68e165da4f/src/parser/mod.rs#L3487

Copy link
Contributor

@iffyio iffyio Aug 3, 2024

Choose a reason for hiding this comment

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

Ah i see, I'm thinking we could still reuse the logic (thinking its currently a bit odd that parse_comma_separated0 is hardcoded to parenthesis while parse_comma_separated is token agnostic as it should be). Seems there's ony a couple of usages of parse_comma_separated0, we could change it to something like parse_comma_separated0(|| {}, Token::RParen)?

Copy link
Contributor Author

@goldmedal goldmedal Aug 3, 2024

Choose a reason for hiding this comment

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

I addressed this in 0763f52 and 5d81654. I share the same logic for brace, parenthesis, and bracket. I think it looks better. Thanks.

let _ = self.next_token(); // consume }
Copy link

@dharanad dharanad Aug 2, 2024

Choose a reason for hiding this comment

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

Nit: ambiguous comments

Copy link
Contributor Author

@goldmedal goldmedal Aug 2, 2024

Choose a reason for hiding this comment

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

I followed the same style as parsing the array literal. Should I enhance them?
https://github.com/sqlparser-rs/sqlparser-rs/blob/d49acc67b13e1d68f2e6a25546161a68e165da4f/src/parser/mod.rs#L1853

Copy link

@dharanad dharanad Aug 2, 2024

Choose a reason for hiding this comment

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

My bad, i interpreted wrongly. This look good.

Ok(Expr::Map(Map { entries: vec![] }))
} else {
let fields = self.parse_comma_separated(Self::parse_duckdb_map_field)?;
self.expect_token(&Token::RBrace)?;
Ok(Expr::Map(Map { entries: fields }))
}
}

/// Parse a field for a duckdb [map]
Expand Down
2 changes: 2 additions & 0 deletions tests/sqlparser_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10282,6 +10282,8 @@ fn test_map_syntax() {
}),
},
);

check("MAP {}", Expr::Map(Map { entries: vec![] }));
}

#[test]
Expand Down