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

Error parsing complex mathematical expression #96

Closed
barrywhart opened this issue Dec 13, 2019 · 2 comments · Fixed by #107
Closed

Error parsing complex mathematical expression #96

barrywhart opened this issue Dec 13, 2019 · 2 comments · Fixed by #107
Assignees

Comments

@barrywhart
Copy link
Member

Input:

SELECT
    COS(2*ACOS(-1)*2*y/53) AS c2
FROM
    t

Output:

$ sqlfluff lint test1.sql
== [test1.sql] FAIL
L:   2 | P:  10 | L006 | Operators should be preceded by a space.
L:   2 | P:  11 | L006 | Operators should be followed by a space.
L:   2 | P:  15 | ???? | Found unparsable segment @L002P015: '(-1)*2*y/53...'
L:   2 | P:  31 | L014 | Inconsistent capitalisation of unquoted identifiers.
L:   4 | P:   5 | L014 | Inconsistent capitalisation of unquoted identifiers.

Interestingly, adding whitespace around the operators also fixes the parsing error. I wonder if two adjacent tokens are being mistakenly parsed as a single token when spaces are missing.

Here's the same SQL with spaces (and no parsing error):

SELECT
    COS(2 * ACOS(-1) * 2 * y / 53) AS c2
FROM
    t

Output:

$ sqlfluff lint test1.sql
== [test1.sql] FAIL
L:   2 | P:  22 | L006 | Operators should be preceded by a space.

I don't know why there's still a warning about needing space around an operator. Can you look into this as well?

@alanmcruickshank
Copy link
Member

alanmcruickshank commented Dec 15, 2019

Sounds like your hunch is very likely. I worry that it might also be related to a change I made in the parsing of negative numbers too. Great to raise this one. I'll pick it up shortly.

I'll also try and post some of my debugging here so you can can get a better feel for how this part of sqlfluff functions. Hopefully it will be useful.

@alanmcruickshank alanmcruickshank self-assigned this Dec 15, 2019
@alanmcruickshank
Copy link
Member

Found the issue. The parser was favouring identifiers over functions. That meant it categorised ACOS as an identifier and then got confused about the brackets. I'm pushing up a fix now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants