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 support #78

Closed
willy610 opened this issue Sep 20, 2020 · 5 comments
Closed

ORDER BY support #78

willy610 opened this issue Sep 20, 2020 · 5 comments
Labels
enhancement New feature or request medium level issue More challenging issue than good first issue

Comments

@willy610
Copy link

Hej,

Sorry for all issues. Not complaining. Love the SQL and in Rust!

But browsing into my swedish testdabase on recipes and it's categories
the output is not ordered on column 2.

(We have the same sort order in Swedish as in any Latin-1 countries)

"select * from category where categoryid > 50 order by 2;"
53 "Fläskkött" ""
54 "Vegetarisk" ""
55 "Pastej" ""
56 "Gratäng" ""
57 "Finskt" ""
58 "Kassler" ""

@panarch panarch added the enhancement New feature or request label Sep 20, 2020
@panarch
Copy link
Member

panarch commented Sep 20, 2020

That's welcome at all, and I'm so pleased about your reportings.

Let's simply rename this issue to "ORDER BY support"

ORDER BY is a type of query which may work best with index, but it still requires to work without index, too.
This issue is implementing ORDER BY without considering indexing stuff.

@panarch panarch changed the title 'Order by' accepted but not evaluated ORDER BY support Sep 20, 2020
@panarch panarch added the medium level issue More challenging issue than good first issue label Sep 20, 2020
@willy610
Copy link
Author

willy610 commented Dec 3, 2020

ORDER BY is an outer cover only useful for the presentation of result. Look into the AST Struct sqlparser::ast::Query A kind of sibling limit

One can assume that the ordering mostly is on a subsets due to joins and where.

The location of an order by is probably in executor/execute.rs and after the select_with_labels

I did an implementation of order by in the benchmark https://github.com/willy610/abcsql.git

(Besides ORDER BY it holds a lot functionality)

It's very similar when using gluesql to take care of requests pattern

let storage = get_storage(how);
let mut glue = Glue::new(storage);
for query in parse(&one_query).unwrap() {
    match &glue.execute(&query).unwrap() {
        Select { labels, rows } => {
           let rows = orderby(labels.to_vec(), rows.to_vec(), &order_by);
           ...

The order by is like

  1. Compile the AST order_by into columnnumber and ask/dsc
  2. Insert each row in a BinaryHeap with Ord implementation using the compiled order_by
  3. Retrieve the rows and return them

I think the orderby.rs can be adapted quit easily into executor/execute.rs

@willy610 willy610 mentioned this issue Dec 3, 2020
@KyGost
Copy link
Contributor

KyGost commented Feb 27, 2021

@willy610

I did an implementation of order by in the benchmark https://github.com/willy610/abcsql.git

Did you do it on the GlueSQL side?
If so, could you share the code or put up a PR?

@willy610
Copy link
Author

willy610 commented Feb 27, 2021 via email

@KyGost
Copy link
Contributor

KyGost commented Feb 27, 2021

@willy610
Thanks :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request medium level issue More challenging issue than good first issue
Projects
None yet
Development

No branches or pull requests

3 participants