Fix: sorting queries by schedule was resulting in a wrong order #4954
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arikfr
changed the title
|
I was about to merge this, but then I realized that this is only half of the solution: this fixes sorting on the frontend, but the backend sorting remains broken (for similar reasons). You can see what I mean in the preview instance: https://deploy-preview-4954--redash-preview.netlify.app/queries?order=schedule&page=1&page_size=5 You will notice that when we limit the page size to 5 queries, a query scheduled to refresh every 30 days shows up before queries scheduled to refresh every minute. I'm not sure how hard it is to fix on the backend, it's possible you can define a We do the same for the |
|
oops, Let me look into this.
Lei
…On Thu, Jun 18, 2020 at 12:46 AM Arik Fraimovich ***@***.***> wrote:
I was about to merge this, but then I realized that this is only half of
the solution: this fixes sorting on the frontend, but the backend sorting
remains broken (for similar reasons). You can see what I mean in the
preview instance:
https://deploy-preview-4954--redash-preview.netlify.app/queries?order=schedule&page=1&page_size=5
You will notice that when we limit the page size to 5 queries, a query
scheduled to refresh every 30 days shows up before queries scheduled to
refresh every minute.
I'm not sure how hard it is to fix on the backend, it's possible you can
define a json_cast_property property (example
<https://github.com/getredash/redash/blob/98a5154345307737c9faa012547c1529a56f2f8d/redash/models/users.py#L102-L104>)
that extracts the interval value and then use it for sorting (mapping
defined here
<https://github.com/getredash/redash/blob/98a5154345307737c9faa012547c1529a56f2f8d/redash/handlers/queries.py#L40-L41>
).
We do the same for the active_at value on the users page, so it's likely
to work. I just don't know what happens when the schedule value is null
instead of a json object.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4954 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APUT5EPVGRIR7H2A6WTG3GTRXHA6RANCNFSM4NXYGCGQ>
.
|
arikfr
approved these changes
Jun 30, 2020
redash/models/types.py
Outdated
Comment on lines
117
to
119
| A SQLAlchemy index property that is able to cast the | ||
| entity attribute as the specified cast type. Useful | ||
| for JSON and JSONB colums for easier querying/filtering. |
There was a problem hiding this comment.
Suggested change
| A SQLAlchemy index property that is able to cast the | |
| entity attribute as the specified cast type. Useful | |
| for JSON and JSONB colums for easier querying/filtering. | |
| A SQLAlchemy index property that is able to cast the | |
| entity attribute as the specified cast type. Useful | |
| for PseudoJSON colums for easier querying/filtering. |
|
Merged 👍 Congrats on your first PR :-) |
andrewdever
pushed a commit
to andrewdever/redash
that referenced
this pull request
Oct 5, 2020
…edash#4954) * fix schedule sorting issue * style change * Update to meet code style. * move the schedule sort to backend * mod comment Co-authored-by: Arik Fraimovich <arik@arikfr.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Description
sorting on schedule was not working. it use schedule which is an object to compare. by default, it's converted to string value. so 1000 is sorted ahead of 60
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)