Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WMS GetCapabilities truncated for a CONNECTIONTYPE POSTGIS #4039

Closed
mapserver-bot opened this issue Apr 4, 2012 · 23 comments
Closed

WMS GetCapabilities truncated for a CONNECTIONTYPE POSTGIS #4039

mapserver-bot opened this issue Apr 4, 2012 · 23 comments

Comments

@mapserver-bot
Copy link

Reporter: jmckenna
Date: 2011/10/04 - 18:09
Trac URL: http://trac.osgeo.org/mapserver/ticket/4039

  • Problem data statement:
    DATA "geom from (SELECT id,dgsid,easting,northing,ST_SetSRID(ST_MakePoint(easting,northing),26957) as geom FROM geophys_logs) as subquery using unique id using srid=26957"
  • GetCapabilities is truncated at that layer:
    ...
    <Layer queryable="1" opaque="0" cascaded="0">
        <Name>geophys-logs</Name>
        <Title>DGS - wells with Geophysical Logs</Title>
        <KeywordList>
          <Keyword>MetadataUrl=http://geodev.dgs.udel.edu/dgir/temp_metadata.htm
l</Keyword>
        </KeywordList>
        <SRS>EPSG:26957</SRS>
        <SRS>EPSG:4326</SRS>
        <SRS>EPSG:4269</SRS>
        <SRS>EPSG:4152</SRS>
        <SRS>EPSG:2776</SRS>
        <SRS>EPSG:3785</SRS>
        <SRS>EPSG:900913</SRS>

  • but specifying "wms_extent" solves it:
    METADATA
       ...
       #'wms_extent' '348360.900000 4015701.800000 582411.600000 4462986.400000'
    END
@mapserver-bot
Copy link
Author

Author: jmckenna
Date: 2011/10/04 - 18:10
Could be related to #3585

@mapserver-bot
Copy link
Author

Author: jmckenna
Date: 2011/10/04 - 19:18
More info on test env:

  • Windows
  • MapServer 6.0.1 (packaged in MS4W)

(good news is that this has been reported with non-MS4W binaries as well: see #3977 comment:4)

@mapserver-bot
Copy link
Author

Author: dmorissette
Date: 2011/10/04 - 19:21
Adding Paul to CC. The issue manifests itself in the WMS GetCapabilities, but I suspect the problem would really take its roots in the PostGIS code. A simple testcase (if possible) might help someone reproduce and fix the issue.

@mapserver-bot
Copy link
Author

Author: jmckenna
Date: 2011/10/04 - 19:25
ah yes, this morning i mentioned the simple test case in a related ticket (3977). and yes creating the simple test case could be an issue...especially if my guesses are right that this is only a Windows issue. :)

@mapserver-bot
Copy link
Author

Author: sholl
Date: 2011/10/06 - 16:20
We had this issue only on ArcSDE-Layers with self-compiled MS from (somewhat older) trunk.

I did not test the mentioned trick with adding WMS_EXTENT to the metadata to solve the problem though.

Too bad I cannot add a testcase since the data is not freely available; but I am highly interested in having a solution for this issue.

Stephan

@gsueur
Copy link

gsueur commented Apr 26, 2012

I'm having a similar issue on Linux, with a Postgis Layer.
METADATA set to
"wfs_metadataurl_href" "http://geosource.grandlyon.fr:80/geosource/srv/fr/csw?SERVICE=CSW&VERSION=2.0.2&REQUEST=GetRecordById&outputSchema=http://www.isotc211.org/2005/gmd&ID=cb5022b7-b5a7-43cd-9617-011ac6a7df71"

is displayed in WMS/WFS Capabilities as :

http://geosource.grandlyon.fr:80/geosource/srv/fr/csw?SERVICE=CSW

It is truncated at the first &

@tbonfort
Copy link
Member

@jmckenna can you provide the mapfile that triggers this (without the data is ok)?

tbonfort added a commit that referenced this issue Jul 30, 2012
when passed an unitialized rectObj, msProjectRect() will try to sample
the rect by going through msProjectRectGrid() which is non needed. This
commit reprojects (minx,miny) and updates the rectObj accordingly. Might
solve #4039 as the failure is happening somewhere inside the
rect reprojection code.
@jmckenna
Copy link
Member

jmckenna commented Aug 7, 2012

@tbonfort I am able to trigger this locally (with Windows, MapServer 6.2.0-beta2, PostgreSQL 9.1.4, PostGIS 2.0). Here is a test case (tiny sql to load 2 points into database included in /data/ folder): http://labs.gatewaygeomatics.com/dl/ticket4039.zip (9KB)

See the included file: commands.txt

@szekerest
Copy link
Member

I've found such issues with earlier versions of MapServer and by adding fflush(NULL) to the end of msCleanup (maputil.c) always solved this problem. May be we should apply such change "officially" if we experience the same with the current version too.

@tbonfort
Copy link
Member

tbonfort commented Aug 8, 2012

@jmckenna I cannot reproduce the hang on linux. However, given where the hang is occuring for you, I suspect that proj is having a problem computing the latlon bounds returned by the postgis driver (which are known to be invalid as that particular driver does not implement this function). I have added a patch that accounts for this situation, could you check if this fixes the issue ?

git remote add tbonfort git://github.com/tbonfort/mapserver.git
git fetch tbonfort
git checkout b4039
# usual clean,build, install, test

before patch:

<LatLonBoundingBox minx="-180" miny="-8.2419e+12" maxx="180" maxy="8.2419e+12" />
<BoundingBox SRS="EPSG:26918"
                    minx="-2.5e+07" miny="-2.5e+07" maxx="2.5e+07" maxy="2.5e+07" />

after:

<LatLonBoundingBox minx="-180" miny="-90" maxx="180" maxy="90" />
<BoundingBox SRS="EPSG:26918"
                    minx="-2.5e+07" miny="-2.5e+07" maxx="2.5e+07" maxy="2.5e+07" />

@tbonfort
Copy link
Member

tbonfort commented Aug 8, 2012

@dmorissette could you please comment on this fix ?

@jmckenna
Copy link
Member

jmckenna commented Aug 8, 2012

@tbonfort I've tested this patch and the workaround does work on Windows.

I wonder if this breaks the WMS spec, as LatLongBoundingBox should be "the minimum bounding rectangle in decimal
degrees of the area covered by the Layer."

I'm +1 for this change.

@dmorissette
Copy link
Contributor

@tbonfort, This issue has come up several times in the past and there seems to be no good fix for it.

  • Ticket PostGIS layer doesn't return a computed LayerGetExtent #3585 has a proposed patch that scans the whole table to compute extents on the fly, but in addition to being slow it seems that there would be an issue with some types of subqueries that would not work with this. One of the options discussed in this ticket is to fallback on the map.extent as default in case the extent is not provided and cannot be computed efficiently.
  • It was proposed in tickets PostGIS layer doesn't return a computed LayerGetExtent #3585, msPOSTGISLayerGetExtent returns infinite extents #287, WMS server returns wrong BoundingBox for PostGIS layers #1851 (and probably a few others) to simply produce a XML warning in the capabilities in this case to encourage the user to set ows/wms_extent for their postgis layers (scanning capabilities for XML warnings is the recommended process in our WMS docs)
  • My take on your patch is that the value of MS_BOUNDS_MAX=25000000 is arbitrary and could fail in cases where the projection is not in meters (projection in feet for instance).
  • How about using the map.extent as default when LayerGetExtents() returns MS_FAILURE? Possibly combined with a msDebug() note or a XML warning? I didn't look in the code to see how practical that would be, but as a fix that may be the best compromise?
  • Maybe for completeness we could also include the option to set ows/wms_extent to "auto" to request on the fly computation of the bounds using the approach from PostGIS layer doesn't return a computed LayerGetExtent #3585? (That would also remove the warning for this layer)
  • Falling back on the map.extent combined with the XML warning (and/or msDebug message) could also help users address @jmckenna's point about the WMS spec requiring the minimum bounding rectangle by encouraging them to set ows/wms_extent to get rid of the warning

@dmorissette
Copy link
Contributor

cc @pramsey and @sdlime, it would be great to get their input on this recurring issue.

@jmckenna
Copy link
Member

jmckenna commented Aug 8, 2012

I really like @dmorissette's suggestions to fall back on map.extent and include an XML warning (I follow these warnings very closely).

@tbonfort
Copy link
Member

tbonfort commented Aug 8, 2012

My intent on this issue was to prevent a segfault/hang from happening, no more no less. The MS_BOUNDS_MAX=25000000 was the value returned previously by the postgis driver, and my patch just treats this special case.

  • producing a warning only: does the wms spec allow a layer without latlonbbox? does removing the block entirely risk breaking some clients that expect to find it ?
  • using map extent + warning : +1
  • ows_extent auto: why not, but that implies a change to the driver vtable api (i.e. force on/off)

@dmorissette
Copy link
Contributor

Unless I'm mistaken, the PostGIS driver used to return infinite values (+/-FLT_MIN,FLT_MAX) in previous releases which was even worse than the 25000000 value. It seemed to me that the 25000000 value was introduced by your patch no? (72217df)

A layer without a LatLonBBox would be invalid if I remember the spec correctly, so I agree with you that using map extent + warning is the best way to go for 6.2.

ows/wms_extent auto could be kept as a future enhancement for 6.4

@tbonfort
Copy link
Member

tbonfort commented Aug 8, 2012

Daniel,
correct, the postgis driver returned +/- FLT_MAX, but that ends up as 2.5e+7 in the capabilities doc so I assumed FLT_MAX==25000000 (which isn't the case, so I'd need to investigate where this 2.5e07 is coming from).

@dmorissette
Copy link
Contributor

Ha! mappostgis.c defines:

#ifndef FLT_MAX
#define FLT_MAX 25000000.0
#endif

@tbonfort
Copy link
Member

Looking at the spec, the latlon extent is inherited from the parent layer (i.e. probably from map.extent) if not included.
Unless there are any objections, I will deactivate the postgis getextent() function which will return an error instead of pseudo-infinite bounds. This will have the consequence of:

  • having a warning message be included in the capabilities docs
  • making the layer inherit the parent layer's extent (i.e. map.extent unless a parent layer already had it's extent specified)
diff --git a/mappostgis.c b/mappostgis.c
index ab88061..4bd65fa 100644
--- a/mappostgis.c
+++ b/mappostgis.c
@@ -65,10 +65,6 @@
 #include "maptime.h"
 #include "mappostgis.h"

-#ifndef FLT_MAX
-#define FLT_MAX 25000000.0
-#endif
-
 #define FP_EPSILON 1e-12
 #define FP_EQ(a, b) (fabs((a)-(b)) < FP_EPSILON)
 #define FP_LEFT -1
@@ -2999,27 +2995,6 @@ int msPostGISLayerGetItems(layerObj *layer)
 }

 /*
-** msPostGISLayerGetExtent()
-**
-** Registered vtable->LayerGetExtent function.
-**
-** TODO: Update to use proper PostGIS functions to pull
-** extent quickly and accurately when available.
-*/
-int msPostGISLayerGetExtent(layerObj *layer, rectObj *extent)
-{
-  if (layer->debug) {
-    msDebug("msPOSTGISLayerGetExtent called.\n");
-  }
-
-  extent->minx = extent->miny = -1.0 * FLT_MAX ;
-  extent->maxx = extent->maxy = FLT_MAX;
-
-  return MS_SUCCESS;
-
-}
-
-/*
  * make sure that the timestring is complete and acceptable
  * to the date_trunc function :
  * - if the resolution is year (2004) or month (2004-01),
@@ -3373,7 +3348,7 @@ int msPostGISLayerInitializeVirtualTable(layerObj *layer)
   layer->vtable->LayerGetShape = msPostGISLayerGetShape;
   layer->vtable->LayerClose = msPostGISLayerClose;
   layer->vtable->LayerGetItems = msPostGISLayerGetItems;
-  layer->vtable->LayerGetExtent = msPostGISLayerGetExtent;
+  /* layer->vtable->LayerGetExtent = msPostGISLayerGetExtent; */
   layer->vtable->LayerApplyFilterToLayer = msLayerApplyCondSQLFilterToLayer;
   /* layer->vtable->LayerGetAutoStyle, not supported for this layer */
   layer->vtable->LayerCloseConnection = msPostGISLayerClose;

@tbonfort
Copy link
Member

How do you guys interpret this sentence from the spec ?

Every Layer shall have exactly one <LatLonBoundingBox> element that is 
either stated explicitly or inherited from a parent Layer. 
  • <LatLonBoundingBox> can be absent from the generated doc as it inherits its parent one
    or
  • <LatLonBoundingBox> must be included in the generated doc, copied from the parent in the present case

@jmckenna
Copy link
Member

I interpret that as we must have a LatLonBoundingBox always (so your second point).

@dmorissette
Copy link
Contributor

I'd interpret it the other way, i.e. "inherited" everywhere else in the WMS spec means that if it's stated in the parent already then there is no need to repeat it, and the child layer inherits the value automatically. I wonder why we always thought that it was required for every layer, perhaps the rule has changed between versions of the spec.

mkofahl pushed a commit to faegi/mapserver that referenced this issue Apr 9, 2013
…er#4039)

when passed an unitialized rectObj, msProjectRect() will try to sample
the rect by going through msProjectRectGrid() which is non needed. This
commit reprojects (minx,miny) and updates the rectObj accordingly. Might
solve MapServer#4039 as the failure is happening somewhere inside the
rect reprojection code.
mkofahl pushed a commit to faegi/mapserver that referenced this issue Apr 9, 2013
return an error instead of invalid extents to avoid peculiar behavior in
proj when reprojecting invalid values. this will also trigger a warning
in capabilities documents.

closes MapServer#4039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants