-
Notifications
You must be signed in to change notification settings - Fork 8
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
Adds ORDER BY
#41
Adds ORDER BY
#41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AMAZING stuff, particular kudos for the great inline docs, the thoughtful unittests, and the super helpful error messages when things fail assertions!! THANK YOU!
I'll merge once you claim your very-much-due credit in the readme / changelog :) (Or I can write that up if you prefer not to!) |
cheers! you're too kind :) i'll add my name and let you merge |
also extra test for
DISTINCT
Adding
ORDER BY
requires some slight changes in the order of operations being applied.Pagination (
LIMIT
+SKIP
) has to wait until all other operations are applied.Did some refactoring on
returns
because it was getting pretty big😅