eloquence-base icon indicating copy to clipboard operation
eloquence-base copied to clipboard

Aggregate is failing with certain db settings

Open ragingdave opened this issue 7 years ago • 6 comments
trafficstars

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 avatar Sep 08 '18 23:09 ragingdave

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.

ragingdave avatar Oct 19 '18 13:10 ragingdave

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

jarektkaczyk avatar Mar 15 '19 07:03 jarektkaczyk

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

ragingdave avatar Mar 16 '19 19:03 ragingdave

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

jarektkaczyk avatar Mar 17 '19 01:03 jarektkaczyk

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.

ragingdave avatar Mar 17 '19 03:03 ragingdave

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

jarektkaczyk avatar Mar 19 '19 06:03 jarektkaczyk