BBox argument handler doesn't work with GROUP BY

Why is the patch a workaround? What should it contain?

Reading http://api.drupal.org/api/views/handlers!views_handler_sort_group_by_num... we should do more indeed.

But according #1806048: BBOX filter not working we should add at least add

  function query($group_by = FALSE) {

to this patch.

Added the method argument as mentioned in #3

Guys, is this fixed in dev version ?

It looks like this patch was not added. So the problem is not fixed.
Currently views_geojson feeds cannot be grouped at all without setting $this->definition['uses grouping'] = TRUE; in views_plugin_style_geojson::init(). This problem still needs work. We need a new patch that applies to current dev. And reenables grouping on this display style to be able to test it at all.

Currently patch #4 has tried to disable grouping. But there are scenarios in which grouping is necessary, notably: server side clustering. See: https://www.drupal.org/project/geocluster
I've reformulated this issue to focus on fixing grouping itself. That should be fixed so each row only returns one (group)item with it's own lat/lon. That way, bounding box behavior should work too.
In case of geoclustering that would be a pseudo cluster: a single point with the cluster count as title and the centroid of the group as coordinates.

@basvredeling - I have a couple of questions on this regarding using SQL geospatial functions. I think that we could use the strategy that disables grouping if no spatial db support exists, but allow it if the site had spatial functions:

1. Do you think that this would be facilitated by enabling Geospatial aggregates like "centroid()" in the Views query itself? Then filtering can be enabled on the aggregate.
2. What about adding the ability to do db-native spatial BBOX queries when supported? MySQL since 5.x has supported basic BBOX and PostgreSQL with PostGIS has it as well. This would be faster most likely because it would enable spatial index support. I have a sandbox module "GeoJoins" that employs this for doing spatial joins between geometries that contains a set of simple tests to determine if OGIS functions are available - https://www.drupal.org/sandbox/robertwb/2202159
3. #2 would also require to use the "centroid()" as well.

Added links to GROUPING issues in View queue.

@robertwb I don't know anything about sql native geospatial functions really. But your approach sounds nice and speedy. However, if I understand correctly, that would mean dropping any solution for bbox filtering if the data is not stored using geospatial sql functionality. Is that assumption correct?
So maybe we should have a 3 way solution:

1. detect if your sandbox module is present. enable aggregation and let this module do all the spatial processing
2. detect if aggregation is enabled and do some custom processing as proposed in #7
3. detect if aggregation is disabled and disable bbox filtering all together

I can work on 2 and 3. In that way we can support both paths.

---- tldr; ----

I'm not happy with the way this whole bbox filtering and server side clustering problem is piling up. It's increasingly a case of matching very specific modules in a very specific setup just to get 1 thing working. And it feels very much like a house of cards. I proposed the above 3 way solution to not introduce any more specific dependencies and just support many use cases.
I'm working on a site for a client where we have a lot of performance issues especially client side, with high numbers of vectors / points on a single map.

1. we're tied to openlayers 2 because of old browser support and module compatibility
2. we can't do generic views openlayers layers because of performance, so I choose a views_geojson with solr, but ...
3. solr doesn't support bbox filtering if your projection is not spherical (at least not in the solr 4), so ...
4. I try to use geocluster but the current module only uses geohash (which doesn't work for the EPSG:28992).
5. I end up in this thread to get simple aggregation to work which has been disabled and messes up my whole performance issue again.

My point is: this module and other modules in the geospatial stack currently have too many exceptions that they don't work in a specific case or only work in a specific configuration. We should really look at hardening the compatibility for: a) different methods of storing geospatial info and b) different Coordinate Reference Systems.

@basvredeling - I will work on #1, as well as adding SQL spatial support checks. Sounds like some sort of plug-in framework might be needed for this, but I do not yet know how to do that stuff. That said, a more robust mapping architecture is in my mind, and that of a number of othe folks - you're not alone in your feeling that there's got to be a better way! I am just about ready to start a thread in the Mapping project that uses an Entity based approach for defining rendering-library-agnostic maps, layers, and sources. I will post back to this thread some time this week.

I've been working on just getting the bbox argument working for search_api_* and getting the code readable.
See if you like where this patch is going.

@robertwb you could add a if (module_exists('geojoin')) {} to the bbox_filter_mysql() method and add some native mysql spatial filtering there. Or you can extend this argument handler completely with your own handler.

With regards to the grouping... I didn't get that far yet. Please give me some feedback on the included patch first.

Okay, here's another concept. I've enabled grouping in views_geojson_views_plugins(). I've also got a concept of grouping working by creating groups by name. This isn't very elegant though. I'd rather retrieve a grouped result set by query. But this works and bbox filtering works too. It's decent enough for someone to base some extra work on. It's not decent enough to commit yet.

Hey @basvredeling - been slammed and jsut saw these last 2 posts. I will give this a look this weekend and get back to you.

Hi all,

I'm waiting for this thread to be fixed so I can release a new beta. What's the status ?

Hi Pol,
the current status is that the patch needs work. It's just a concept right now. I didn't get any comments about the new direction of the patch.
Do you want me to push it forward to a reviewable patch?
To sum up the direction of the patch:

• Grouping is reenabled.
• Grouping works post-factum: the result set is completely fetched, results are grouped after the query is completed. (I know, this is ugly, but we're working with different views backends, and search api / solr doesn't support GROUP BY).
• Grouping works by bundling results by the assigned name / title field of the group.
• A primitive centroid is calculated by getting the mean of all latitudes and longitudes within the group.
• Bounding Box filtering works again because it has a single point to filter on (the primitive centroid mentioned above).

Another patch. Simplified this time. The BBOX argument handler is not used because we are grouping after the whole result set is fetched. It is therefore independent of storage type as discussed in #9 #10 #11 & #12. This is not the optimal solution but at least it works reliably.

Please have a look at not just the code but also the configurability.
You need to create a geojson data layer in views. Enable grouping. And group by a property of the fieldable entity. For instance:

• Make a set of nodes with a geofield and assign a vocabulary.
• Create 2 taxonomy terms with a name and description.
• Set half the nodes to 1 taxonomy term, set the other half to another taxonomy term.
• Configure the views layer to group by taxonomy term name.
• Set the feature name and feature description of the map view display to match taxonomy term name and taxonomy term description.

The resulting map will show 2 points. Centered on average lat / lon of the grouped values.
If you assign this data layer to lower zoom levels (zoomed out) and another data layer without grouping to higher zoom levels (zoomed in) you can display a map with grouped features when zoomed out, and individual features when zoomed in.

An alternative approach using the spatial aggregate st_UNION. It relies on 2 things, however:
1) You must patch views to accept new aggregates (works, but not status RTBC): https://www.drupal.org/node/1359298
2) The output of the aggregate is only compatible with "Use Full Geometry" setting, so, getting the centroid from GeoPHP/Geofield is not possible as of now.

Regardless, I think this is the way to go with this ultimately. Once applying the patch from 1359298 adding the(any) aggregate with this approach requires only the following code:

function [module name]_views_plugins() {
  return array(
    'query_aggregate' => array(
      'st_union' => array(
        'title' => t('Spatial Union'),
        'method' => 'views_query_default_aggregation_method_simple',
        'handler' => array(
          'argument' => 'geofield_handler_argument',
          'filter' => 'geofield_handler_filter',
          'sort' => 'geofield_handler_sort',
        ),
      ),
    ),
  );
}

As much as I agree with this being the more elegant solution, it does have a lot of liabilities:
1) requires a patch on other module
2) requires mysql spatial extension
3) only works with full geometry
4) only works with geofield, not latlon or wkt

Is there any way to make this a more universal solution?

I think that a method of detection and fall back to manual methods is unavoidable for the short term. To be clear though, the spatial extensions are part of the base install >= MySQL 5.6. As for the views patch, I agree with you. The problem I think is that the Views crew usually don't commit patches for D7 unless they are backports from D8 - I am working on a D8 version now. It would help the cause however if you or anyone else could test the view patch.

I am wondering if this is partly a GeoField issue, that is, aggregates are not properly handled in all cases, so I created an issue Support Spatial Aggregates in Views - so linking it here.

To take this thread a bit further off-topic, the SQL aggregate approach includes some even more compelling functional options. For PostgreSQL installs, the newer version of PostGIS (>= 2.2.0) include some clustering aggregate functions out of the box:

• http://postgis.net/docs/manual-2.2/ST_ClusterWithin.html
• http://postgis.net/docs/manual-2.2/ST_ClusterIntersecting.html

ST_ClusterWithin() takes as argument a distance value, making it ideal for another use case of the SQL aggregate I think. I don't think MySQL possesses ST_ClusterWithin() yet. While the necessity of specific platform and package support is of concern, when we are talking about integrating solr or other into the solution, this seems to me to be more of a power-user type of application anyhow.

@robertwb would you mind retrofitting the spatial support via a new issue and just solving the problem at hand fttb?

Further scope increase is only blocking a solution for our current troubles. I'd prefer the detection and fallback method to be part of an issue similar to #2660312: Support Spatial Aggregates in Views. Call it: "Support Spatial MySQL", perhaps? This can be worked out and wait on eventual commit of the views patch. We can simultaneously work out a fallback. But include a patch in the mean time which solves a problem now. We could even add a comment for it to be placed into a fallback in the future.

@basvredeling -- sorry to move things off topic with the spatial aggregates implementations. I agree, solving the immediate issue is the way to go. But I think that what you are solving is actually a geofield issue, that is, none of the meta-data that a geofield normally possesses, lat, lon, etc., exist once an aggregate is performed (any aggregate that is).

Your solution solves both the issue with the missing meta-data (by calcing lat and lon), and also performs a spatial centroid function (if I'm reading it right) -- but they are separate issues: the missing meta data is needed for supporting any aggregates, and the calculation of a centroid is the actual fallback function. I think that the first should be a patch to GeoField, and the second should be a patch to Views (in the form of an aggregate).

I think your patch obviously solves the stop-gap case that you need, but I don't know that we should advocate it be committed since I think the problems lie outside of Views Geojson? Also, if I read your code right it is limited in scope to points only, or at least if it calculates a centroid for another geometry type it is an approximation based on the first point in the geometry?

I've added a patch to #2747493: Search API bbox support which creates a subclass for search api bbox filtering.

I think your patch obviously solves the stop-gap case that you need, but I don't know that we should advocate it be committed since I think the problems lie outside of Views Geojson?

I think @robertwb has got a point here. And I'd suggest subclassing the generic handler here too. So instead of all the if statements within the views_geojson_bbox_argument::query() method we just create a subclass for most common setups: db supporting spatial aggregates, and a simple subclass for the fallback post-facto grouping.

With regards to patching geofield: the current metadata of the geofield contain top/right/bottom/left coordinates. The allow for quick centroid calculations (albeit unweighted), so we can both intersect the bbox with the approximate area of a polygon. Or we can do quick comparisons between bboxes and points / polygon-centroids. So I don't think a patch is needed there.