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 rework #39

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Order by rework #39

wants to merge 10 commits into from

Conversation

transistive
Copy link
Collaborator

Hello once again 👋

I just found out ORDER BY allows for order modifier for each property. This is possible:
ORDER BY x.x ASC, x.y DESC

For this reason I had to rework the OrderByClause to use Order objects and not worry about the order itself.

Looking forward to your feedback!

@marijnvanwezel
Copy link
Collaborator

Please make sure tests are passing for PHP 7.4, 8.0 and 8.1.

@transistive
Copy link
Collaborator Author

Looks like I did a whoopsie. I fixed it, so there aren't any regressions on the Query object

Copy link
Collaborator

@wgevaert wgevaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overal looks very good, nice work! I have some small remarks

src/Clauses/OrderByClause.php Outdated Show resolved Hide resolved
src/Order.php Outdated Show resolved Hide resolved
@marijnvanwezel
Copy link
Collaborator

Please make sure the checks are passing.

@marijnvanwezel
Copy link
Collaborator

I have been looking at this, and I cannot seem to find any documentation on this. I tried it out, and it did seem to work. Is this documented anywhere?

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

Successfully merging this pull request may close these issues.

3 participants