Closed (fixed)
Project:
Geocoder
Version:
8.x-4.29
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
29 Jul 2025 at 11:21 UTC
Updated:
16 Aug 2025 at 12:29 UTC
Jump to comment: Most recent
Problem/Motivation
We first noticed this issue when our database reported it was full and we saw that the node_revision__field_location table took up 500MB.
After some investigation we discovered this was happening because when an address is geocoded in the queue it is incorrectly marked as needing resaving which then calls the presave hook and requeues that item. Further investigation lead me to FieldItemListInterface::equals in _geocoder_field_process. The problem there is that longitude and latitude values are stored in the database as strings while they are geocoded as floats and so in this instance they are reported as unequal (probably due to precision).
Steps to reproduce
Geocode an address in the United Kingdom With some more investigation the error happens when the latitude and longitude happen to be a float can't be represented correctly in a 64 bit double (stored using the address module) into a geolocation field with queuing on in the settings and using google maps as the provider. Then run cron and see that the entity is processed and requeued in the queue table
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork geocoder-3538539
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
aurora-norris commentedComment #3
aurora-norris commentedI can fix it by patching core with the fix for https://www.drupal.org/i/2925972 but I feel like there should be a less invasive way of fixing this.
Comment #4
aurora-norris commentedComment #5
aurora-norris commentedI've found better way to fix it, if just before you set the new field you get the current field and compare that both
GeolocationItemwill have data stored as floats and because they get their data from the same source they are exactly equal.If a maintainer thinks this is a good way to solve this issue then I can make a merge request.
Comment #6
itamair commentedHi @aurora-norris thanks for reporting and inspecting all this, so deeply.
Actually I am already inspecting and debugging this ... and indeed it looks that the GeocoderField @QueueWorker $entity->save() here:
https://git.drupalcode.org/project/geocoder/-/blob/8.x-4.x/modules/geoco...
is retriggering a new geocoder_field_entity_presave, at least once ... and then not a further time, as it looks the second time the $result === FALSE and then the $entity is not being resaved recursively ...
Thus I was thinking we should mark/flag the saved $entity in some way as saved by the @QueueWorker and stop/skip all the new geocoder_field_entity_presave processing in that case.
I am thinking to inspect and implement this approach.
What you think?
Feel free to open your MR if you ended up to a better fix, so that I could review it, and compare, eventually.
Feel also free to PM me (itamair) on Drupal slack for more immediate feedback.
Comment #8
itamair commented@aurora-norris it looks that I was quicker and opened a MR !70 with a solution that relies on drupal_static cache so to mark the entity as being saved by the geocoder queue and stop any further processing in the geocoder_field_entity_presave.
That looks pretty logical and solid to me, and looks properly fixing this issue and stop further redundant and infinite entity revisions generations.
Please QA and Review ... and let me know if some more work is needed on this (and why eventually).
Comment #9
aurora-norris commented@itamair
I've tested your merge request and it fixes my issue and processes the addresses that were being resaved every cron run.
It looks like the better and more correct solution compared to mine.
The code looks good and I've traced what happens on a normal entity save (where the entity is queued) and on a save by the QueueWorker and the new code does exactly what its meant to do.
I'll make a MR with my solution in a minute so you can compare them because both our solutions fix the issue at different levels.
Comment #11
aurora-norris commentedMade a MR with my solution, feel free to not use.
Comment #13
itamair commentedClosing this as Fixed, with MR70! merged. Thanks @aurora-norris ... crediting you for this opportune catch.
Comment #14
ressaThis was a beautiful issue to find by chance, while checking changes in the latest release -- both the great detective work and thorough issue report with plenty of details by @aurora-norris, and @itamair coding and releasing a solution in less than a day. It's quite impressive, and I am very grateful to both of you.