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

should Call.Name be normalized to lower case? #2471

Closed
neonstalwart opened this issue May 1, 2015 · 3 comments · Fixed by #2473
Closed

should Call.Name be normalized to lower case? #2471

neonstalwart opened this issue May 1, 2015 · 3 comments · Fixed by #2473

Comments

@neonstalwart
Copy link
Contributor

works:

select percentile(value, 95) from foo

doesn't work ("error": "expected one argument for PERCENTILE()"):

select PERCENTILE(value, 95) from foo

there seems to be quite a few places in the code that compare to the lowercase version of Call.Name. would it be better to normalize Call.Name to be lowercase when it's created or even to store the original as well as a normalized?

@otoolep
Copy link
Contributor

otoolep commented May 1, 2015

All functions should be case-insenstive. It's on our list to do, but we need to look into solving it at a global level (perhaps at the parser or scanner level) and try to avoid fixing it for individual functions if possible.

@toddboom
Copy link
Contributor

toddboom commented May 1, 2015

@neonstalwart i think this is a good idea. i'm kicking the build on your PR now.

@neonstalwart
Copy link
Contributor Author

ok, so my first thought is to change things like

&Call{Name: expr.Name, Args: args}

to

&Call{Name: strings.ToLower(expr.Name), Args: args}

would that be a good start?

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.

3 participants