ethercreative / simplemap

A beautifully simple map field type for Craft CMS.
Other
135 stars 48 forks source link

After update to CraftCMS 4.9.5 mapsCalculatedDistance Column not found #391

Closed viesrood closed 5 months ago

viesrood commented 6 months ago

After I update from CraftCMS 4.9.4 to 4.9.5, I get the following error: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'mapsCalculatedDistance' in 'having clause'.

viesrood commented 6 months ago

The error happens when filtering out entries based on location and radius.

matthewstick commented 5 months ago

I'm seeing the same issue

matthewstick commented 5 months ago

Unfortunately the issue makes the plugin unusable, since the main reason we add this plugin is to search by location and radius. Hopefully it can be fixed soon? May look to downgrade Craft CMS if not.

viesrood commented 5 months ago

I have already downgraded Craft CMS because a significant part of my website relies on search by location and radius. I hope this can be fixed soon.

matthewstick commented 5 months ago

Same here. And confirmed: Downgrading to Craft 3.9.4 fixed the issue. Something happed in Craft 3.9.5 specifically to break this.

samhibberd commented 5 months ago

Same here. Specifically on the COUNT query for us, it's related to the way craft recently changed the way it handles scalar queries and further update in 4.9.5 changed the way the SELECT value was handled see changelog:

Scalar element queries now set $select to the scalar expression, and $orderBy, $limit, and $offset to null, on the element query. (https://github.com/craftcms/cms/issues/15001)

Exactly the same issue here: https://github.com/doublesecretagency/craft-googlemaps/issues/111

samhibberd commented 5 months ago

I'm no mysql expert and but having a quick look over this, moving the query condition that checks the radius into a WHERE condition rather than the HAVING condition currently used looks to fix this specific issue. Not knowing the plugin, i'm not sure if there are any knock on implications of this, but it does't look like the HAVING method works off any groupings, so WHERE should cover it.

Not throughly tested the impact on performance, but appears minimal in the very specific scenario i have tested in.

So removing this block: https://github.com/ethercreative/simplemap/blob/07594b09d4bb5a889949188999add14c393de796/src/services/MapService.php#L385-L388

And adding the WHERE condition $query->subQuery->andWhere($search . ' <= ' . $radius); to the subQuery (basically replicating what is done for postgres):

$query
    ->subQuery
    ->addSelect($search . ' as [[mapsCalculatedDistance]]')
    ->andWhere($restrict)
    ->andWhere($search . ' <= ' . $radius)
    ->andWhere([
        'not',
        ['[[' . $table . '.lat]]' => null],
    ]);

Any thoughts @Tam?

shornuk commented 5 months ago

I've got this as well. I didn't catch it before releasing and appeared to have a spike in Google API usage during the period I updated too which I'm investigating.

samhibberd commented 5 months ago

Hi @Tam sorry to chase this one, appreciate you are busy, but this is pretty critical as we are stuck unable to update Craft beyond 4.9.4.

I am happy to put in a PR to try and patch, but having very limited understanding of the location query sql logic, I would be wary of inadvertently causing issues elsewhere.

It would be great if you could take a look at my thoughts above (https://github.com/ethercreative/simplemap/issues/391#issuecomment-2139664772)

I have also reached out to Craft to see if they have any insight in how this should be implemented as a result of the changes in 4.9.5.

samhibberd commented 5 months ago

Thanks for jumping on this @Tam

Not tested yet, but won’t this impact all scalar queries, any query that relies on the Query::queryScalar() method, in addition to count() thats things like exists() min() max() average() sum(). I know we rely on exists() a fair bit.

samhibberd commented 5 months ago

Just following up on this @Tam, the fix does work on the exists() query as the scalar expression used does include the mapsCalculatedDistance select, although the same issue remains for min() max() average() and sum() queries.

brandonkelly commented 5 months ago

Craft 4.10.2 and 5.2.2 are out with a fix for this.