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

Order by string column fails with top_n_asc not supported for type ScalarStr #132

Open
jrmuizel opened this issue May 24, 2020 · 4 comments

Comments

@jrmuizel
Copy link

With https://flatteningthecurve.herokuapp.com/data/canadatesting

$ ./repl --load /tmp/canada_testing.csv
locustdb> select * from default where province = 'Ontario' order by 'date'
Some assumption was violated. This is a bug: top_n_asc not supported for type ScalarStr
@cswinter
Copy link
Owner

cswinter commented May 24, 2020

This query is asking to order by the constant string 'date'. I'll have to think about what the semantics of that should be, but it's likely not what is intended. A more useful query would be select * from default where province = 'Ontario' order by date but for some reason this fails to parse. I'm not sure why this is yet, the tests cover queries that are all but identical and work just fine.

  • Add new test case that triggers the parse error
  • Find and fix bug that prevents the query from getting parsed
  • Figure out and implement the semantics of ordering by constant expression or reject such queries with meaningful error messages

@jrmuizel
Copy link
Author

I actually meant select * from default where province = 'Ontario' order by "date" but that doesn't work either. I guess other databases let you quote column names by LocustDB treats it as a column of null.

select * from default where province = 'Ontario' order by date probably doesn't work because of https://github.com/andygrove/sqlparser-rs/issues/168.

@cswinter
Copy link
Owner

I think I found the bug, the double quotes are not getting removed by either the parser or LocustDB so they end as part of the column name which is why LocustDB doesn't find the column as assumes it as null. This should be an easy fix, PR incoming.

@cswinter
Copy link
Owner

The query select * from default where province = 'Ontario' order by "date" now succeeds on master, I'm kicking off a new release that has the fix (v0.3.3).

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

No branches or pull requests

2 participants