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

Aggregate is failing with certain db settings #4

Open
ragingdave opened this issue Sep 8, 2018 · 6 comments
Open

Aggregate is failing with certain db settings #4

ragingdave opened this issue Sep 8, 2018 · 6 comments

Comments

@ragingdave
Copy link

In the current Query Builder instance in this package, the aggregate method doesn't remove things like order at least like the latest laravel version does to remove errors around full group by.

I can run the same query twice using this package enabled and without and the resulting queries look like:

With:

select count(*) as aggregate from "manufacturers" order by "id" desc

Without:

SELECT count(*) as aggregate FROM "manufacturers"

This causes the query to fail on servers where this option is enabled, whereas base installs of laravel, can function properly with this option.

For reference, https://github.com/laravel/framework/blob/master/src/Illuminate/Database/Query/Builder.php#L2421

@ragingdave
Copy link
Author

Is this package still being maintained at all? I would think that a month's time would be enough to at least comment on things? The last change I suggested last time took close to 3 months (maybe more) to even have a response of some kind.

@jarektkaczyk
Copy link
Owner

thanks for feedback 👍
this method cannot be simply removed as original laravel's behavior works differently when we have subquery used in searchable

@ragingdave
Copy link
Author

Shouldn't this be something that the Searchable package has then? If I'm not using it why should usage of something unrelated break?

@jarektkaczyk
Copy link
Owner

It shouldn't break indeed. Can you provide a sample code to reproduce exactly the issue?

@ragingdave
Copy link
Author

I don't recall specifically what the code was but based on the resulting query I had here:

Model::orderBy('id')->count()

This would work in laravel but not under this package.

@jarektkaczyk jarektkaczyk reopened this Mar 19, 2019
@jarektkaczyk
Copy link
Owner

I will give it a shot and make a fix where necessary! 🥂

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

No branches or pull requests

2 participants