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

Smarter sorting that doesn't keep nesting the sorts #135

Closed
simonw opened this issue Jul 1, 2021 · 4 comments
Closed

Smarter sorting that doesn't keep nesting the sorts #135

simonw opened this issue Jul 1, 2021 · 4 comments
Labels
enhancement New feature or request

Comments

@simonw
Copy link
Owner

simonw commented Jul 1, 2021

Currently if you click "sort ascending" and then "sort descending" in the cog menu you get SQL something like this:

select * from (
  select * from (
    select id, slug, link_url, link_title, via_url, via_title, commentary, created, metadata, search_document, import_ref, card_image from blog_blogmark
  ) as results order by "link_title"
) as results order by "link_url" desc

Each sort wraps the previous sorted query in another nested query.

We can do better than this! Sorting really complex queries with with (...) in them and suchlike is hard, but we can at least notice if the query we are re-sorting ends with order by X already and replace that clause rather than wrapping the whole thing.

@simonw simonw added the enhancement New feature or request label Jul 1, 2021
@simonw
Copy link
Owner Author

simonw commented Jul 1, 2021

I can do this with a regular expression. I'm going to assume that the column name is double-quoted, since that's the generated SQL I'll be using for order by clauses, so the expression can be quite simple:

re.compile(' order by "[^"]+"( desc)?$')

@simonw
Copy link
Owner Author

simonw commented Jul 1, 2021

This looks like it works:

In [4]: sql = """select * from (
   ...:   select * from (
   ...:     select id, slug, link_url, link_title, via_url, via_title, commentary, created, metadata, search_document, import_ref, card_image from blog_blogmark
   ...:   ) as results order by "link_title"
   ...: ) as results order by "link_url" desc"""
In [21]: r = re.compile('(^.*) order by "[^"]+"( desc)?$', re.DOTALL)
In [24]: print(r.match(sql).group(1))
select * from (
  select * from (
    select id, slug, link_url, link_title, via_url, via_title, commentary, created, metadata, search_document, import_ref, card_image from blog_blogmark
  ) as results order by "link_title"
) as results

@simonw
Copy link
Owner Author

simonw commented Jul 1, 2021

I'm going to attempt match the regex. If it matches, I'll use the captured group and add the order-by on the end. If it doesn't match, I'll wrap the whole thing in a sub-query and stick the order by on that.

@simonw simonw closed this as completed in b88350f Jul 1, 2021
simonw added a commit that referenced this issue Jul 1, 2021
@simonw
Copy link
Owner Author

simonw commented Jul 1, 2021

Here are the tests I wrote for this:

@pytest.mark.parametrize(
"sql,sort_column,is_desc,expected_sql",
(
(
"select * from foo",
"bar",
False,
'select * from (select * from foo) as results order by "bar"',
),
(
"select * from foo",
"bar",
True,
'select * from (select * from foo) as results order by "bar" desc',
),
(
'select * from (select * from foo) as results order by "bar" desc',
"bar",
False,
'select * from (select * from foo) as results order by "bar"',
),
(
'select * from (select * from foo) as results order by "bar"',
"bar",
True,
'select * from (select * from foo) as results order by "bar" desc',
),
),
)
def test_apply_sort(sql, sort_column, is_desc, expected_sql):
assert apply_sort(sql, sort_column, is_desc) == expected_sql

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

No branches or pull requests

1 participant