Skip to content
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

feat: Array parsing support #604

Merged
merged 13 commits into from
Mar 7, 2022
Merged

feat: Array parsing support #604

merged 13 commits into from
Mar 7, 2022

Conversation

nanderstabel
Copy link
Contributor

What's changed and what's your intention?

Draft pull request resolving adding support for ARRAY value expression parsing #516. Both the ARRAY[1, 2, 3] and {1, 2, 3} syntax are supported.

  • New Expr variant `Array(Vec) is added including display trait.
  • parse_comma_separated_uniform creates a vector of Expr with uniform value types.
  • Added two functions: parse_bracketed_exprs and parse_braced_exprs similar to the alredy existing parse_parenthesized_exprs function. I did also remove Token::RBracket from the get_next_precedence() function since it was messing with the nested parsing. As far as I could see it was not used anywhere else.
  • Limitation: empty arrays are not handled, mirroring the current ROW implemention. @neverchanje please let me know if this is still needed.

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

@nanderstabel nanderstabel changed the title Array parsing feat: Array parsing support Mar 1, 2022
@codecov
Copy link

codecov bot commented Mar 1, 2022

The author of this PR, nanderstabel, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at [email protected] with any questions.

@skyzh
Copy link
Contributor

skyzh commented Mar 1, 2022

The misc-check failed due to network failure, don't worry about that.

@neverchanje neverchanje added the good first issue Good for newcomers label Mar 1, 2022
/// Parse a comma-separated list of 1+ items accepted by `F` with uniform value types
pub fn parse_comma_separated_uniform<T, F>(&mut self, mut f: F) -> Result<Vec<T>, ParserError>
where
F: FnMut(&mut Parser) -> Result<T, ParserError>,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can leave type validation to the optimizer. Because you may be unable to determine the type of a very complex expression, e.g. ARRAY[1+2*3, 3.0] could be valid as type LIST<Decimal>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I see, I was thinking too complicated. Will use parse_comma_separated instead 👍

@neverchanje
Copy link
Contributor

Limitation: empty arrays are not handled, mirroring the current ROW implemention. @neverchanje please let me know if this is still needed.

Empty row in the format of ROW() is allowed in the parser. You can test the syntax validity against postgresql.

@nanderstabel
Copy link
Contributor Author

If I am running this test I'm getting parsererror though:

SELECT ROW()::foo;
---
SELECT CAST(ROW() AS foo)
=>
Query(Query { with: None, body: Select(Select { distinct: false, projection: [UnnamedExpr(Cast { expr: Row(), data_type: Custom(ObjectName([Ident { value: "foo", quote_style: None }])) })], from: [], lateral_views: [], selection: None, group_by: [], having: None }), order_by: [], limit: None, offset: None, fetch: None })

Error:

The input SQL:
  SELECT ROW()::foo;
Unexpected failure:
  sql parser error: Expected an expression:, found: )


thread 'run_all_test_files' panicked at '1 test cases failed', sqlparser/tests/test_runner.rs:115:9

In a similar way for ARRAY currently ARRAY[]::foo[] will break.

@neverchanje
Copy link
Contributor

Ok, so that's a bug.

postgres=# create type foo as ();
CREATE TYPE
postgres=# SELECT ROW()::foo;;
 row
-----
 ()
(1 row)

SQLs as above are quite well-supported in PG. Fix it later.

@nanderstabel nanderstabel marked this pull request as ready for review March 2, 2022 16:37
Copy link
Contributor

@neverchanje neverchanje left a comment

Choose a reason for hiding this comment

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

rest lgtm

Comment on lines 1972 to 1991
pub fn parse_bracketed_exprs(&mut self) -> Result<Vec<Expr>, ParserError> {
if self.consume_token(&Token::LBracket) {
let exprs = self.parse_comma_separated(Parser::parse_expr)?;
self.expect_token(&Token::RBracket)?;
Ok(exprs)
} else {
self.expected("an array of expressions in brackets", self.peek_token())
}
}

pub fn parse_braced_exprs(&mut self) -> Result<Vec<Expr>, ParserError> {
self.prev_token();
if self.consume_token(&Token::LBrace) {
let exprs = self.parse_comma_separated(Parser::parse_expr)?;
self.expect_token(&Token::RBrace)?;
Ok(exprs)
} else {
self.expected("an array of expressions in braces", self.peek_token())
}
}
Copy link
Contributor

@neverchanje neverchanje Mar 3, 2022

Choose a reason for hiding this comment

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

These two functions are duplicated (as well as parse_parenthesized_exprs).

    pub fn parse_braced_exprs(&mut self) -> Result<Vec<Expr>, ParserError> {
      parse_token_wrapped_exprs(self, &Token::LBrace, &Token::RBrace)
    }

    fn parse_token_wrapped_exprs(&mut self, left: &Token, right: &Token) -> Result<Vec<Expr>, ParserError> {
        self.prev_token();
        if self.consume_token(left) {
            let exprs = self.parse_comma_separated(Parser::parse_expr)?;
            self.expect_token(right)?;
            Ok(exprs)
        } else {
            self.expected("an array of expressions in {} and {}", self.peek_token(), left, right)
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to remove those duplicated functions completely (including parse_parenthesized_exprs) and replaced them with parse_token_wrapped_exprs.

…rse_token_wrapped_exprs(left, right) function
@nanderstabel nanderstabel merged commit 3af0840 into main Mar 7, 2022
@nanderstabel nanderstabel deleted the array-parsing branch March 7, 2022 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants