PostGIS layer doesn't return a computed LayerGetExtent

[guillaume]

msPostGISLayerGetExtent function is using hard coded values for a result. A TODO is present : TODO: Update to use proper PostGIS functions to pull extent quickly and accurately when available.

I'm willing to fix this in order to have precise BBOX computations in GetCapabilities without having to set any wms_extent or EXTENT for the LAYER definition.

An OWS interface approach would be to have msPostGISLayerGetExtent return MS_FALSE and then use and transform Map's extent for layer extent.

But I guess fixing this directly in msPostGISLayerGetExtent would be great. I'd be glad to help, but my small C and mapserver hacking experience require hints.

[guillaume]

Patch to get correct computation of extent for a postgis layer

[guillaume]

I succeed in writing a patch, even if it was my first C experience. Tested with QGis which is now able to display a postgis layer which doesn't have an EXTENT explicitly set. Still work is extent or wms_extent is set though. Please review, I did my best but it may be not perfect. Regards,

Guillaume

[pramsey]

The patch looks structurally fine, but think about what will happen to MapServer in the event that you run this patch on a layer with 10000000 features in it. It will take 2 minutes to return the WMS capabilities file. And if you get two requests for capabilities simultaneously all bets are off.

The reason there is no extent code is that there is no "easy, fast, correct" solution to the problem. You can get a fast answer with estimated_extent(), but it won't be correct (and occasionally will return null, if there are no statistics available). You can get the correct answer with the approach you have taken, but it will be cripplingly slow in many cases. Since most data extents don't actually change very often, I have long thought that the "least worse" solution is just the current one: have people set their extents manually with wms_extent.

[pramsey]

Congratulations on your first C patch, BTW. Fun, isn't it?

[guillaume]

Very fun indeed, mostly the * and malloc parts... as well as debugging of course ! The problem is most people don't know that they have to set a wms_extent to have a correct bbox for their PostGIS layer. They don't have to do that for shapefiles and even Oracle Layer has got a computation. On the top of that, the GetCapabilities doesn't echo any warning about it. Maybe sending back MS_FAILURE, which causes a warning message in the capabilities would be more useful for people. I globally think that the layer extent should be the map extent if no specific extent is present in the LAYER block. But that would lead to depreciate what has been done on shapefiles and others. I don't know what to think. Maybe a test if layer->type == MS_POSTGIS_LAYER && layer->extent == NULL && layer->metadata->wms_extent == NULL then take the map's extent for this layer's extent ? Best regards Guilaume

[pramsey]

I think a warning in the XML comments would be a good idea. I think having the extent calculation by default would lead to lots of "why is mapserver slow" questions, but having an XML warning would probably lead people setting up their WMS for the first time to recognize the manual extent issue and address it (that has certainly been my workflow in doing Mapserver WMS -- try a config and read the capabilities and remove the warnings).

I'm less certain of any automatic filling in of extents... though I suppose your approach is no worse than the current Inf/-Inf extent. Except, what happens in the care where all layers are postgis layers? I can't recall, is MAP.EXTENT a required mapfile field?

[guillaume]

MAP.EXTENT is not mandatory, but as the doc says : EXTENT [minx] [miny] [maxx] [maxy]

The spatial extent of the map to be created. In most cases you will need to specify this, although MapServer can sometimes (expensively) calculate one if it is not specified.

We maybe need another test here, and if no extent is specified nowhere, send the user on the moon. Another idea : can't we add a test on the number of records, and if it is reasonnable (<100000 ? <1000000 ?) compute the precise extent, if not send back MS_FAILURE ?

[pramsey]

Number of records is another thing that is expensive to calculate exactly (select count(*) from table forces a seqscan), though statistics on it are probably more available than on spatial extent (but any appeal to statistics has to handle the case where they have not been gathered yet).

Fall back to Inf/-Inf if there's nothing else available, I think.

[guillaume]

Inf/-Inf is leading to misunterstanding. As the values are reprojected in the layer's projection, you see values in getCapabilities which seem computed (different values for min and max etc). When QGIS doesn't show your layer because the view's bbox doesn't intersect the layer's miscomputed bbox, you can get crazy pretty quickly. I would suggest to send back MS_FAILURE which has the huge advantage of displaying a warning in the getCapabilities and leads to better behaviour for the client and better understanding for the user.

[pramsey]

I started implementing and then realized there is a whole other case which messes things up, the "the_geom from (select foo from bar) as subq" case, where the data source isn't a table at all, but a subquery. You can use estimated_extent() to get the extent of tables quickly, but it does nothing for subqueries.

[dmorissette]

Bump... this is still an issue in MapServer 6.x and trunk: ​http://lists.osgeo.org/pipermail/mapserver-users/2011-September/070003.html