One-pass query implementation (RFC 52)

[sdlime]

This ticket will be used to track the implementation of one-pass queries as discussed in RFC 52).

[sdlime]

Implemented a new version of msLayerWhichItems() that works from an item list gotten from the driver (via msLayerGetItems()). One drawback is that since we know what we're looking for in logical filters and expressions we don't know what we're missing so as written we can't detect typos in those strings. This is because certain things that look like item tags are not:

EXPRESSION ([Area] > 50 && '[Name]' =~ /^[A-Z]/)

The [A-Z] is part of a regex. The old version of this code would throw an error that 'A-Z' was not a valid string. In this version this expression works. Mis-spellings or missing items elsewhere are not detected.

I also simplified the function call. Either get all the items (for query) or just those specified within the LAYER definition. No more deciding if one will classify, label or whatever, it just wasn't worth the trouble and made for convoluted code.

Committed in trunk, r9143.

Steve

[tomkralidis]

FYI when I build with trunk, I get a MapScript build error:

make[1]: Entering directory `/home/tkralidi/foss4g/mapserver/svn/trunk/mapserver/mapscript/php3'
gcc -fPIC -g  -fPIC -Wall   -DUSE_PHP_REGEX -DCOMPILE_DL=1 -DPHP4 -DNEED_NONBLOCKING_STDERR      -DUSE_WMS_LYR -DUSE_WFS_LYR -DUSE_SOS_SVR -DUSE_LIBXML2 -DUSE_CURL -DUSE_WCS_SVR -DUSE_WFS_SVR -DUSE_WMS_SVR -DUSE_MING_FLASH   -DUSE_POSTGIS  -DUSE_GDAL -DUSE_OGR -DUSE_GEOS   -DUSE_PROJ -DUSE_EPPL  -DUSE_AGG   -DUSE_PDF  -DUSE_GD_GIF -DUSE_GD_PNG -DUSE_GD_JPEG -DUSE_GD_WBMP -DUSE_GD_FT -DGD_HAS_FTEX_XSHOW -DGD_HAS_GDIMAGEGIFPTR -DGD_HAS_GETBITMAPFONTS -DUSE_ICONV -DUSE_ZLIB  -I/home/tkralidi/foss4g/mapserver/svn/trunk/mapserver    -I/usr/include/libxml2  -I/usr/local/include   -I/usr/include  -I/usr/local/include -I/usr/local/include  -I/usr/local/include  -I/usr/include/freetype2   -I/usr/local/include -I/usr/include  -I/usr/include  -I/usr/local/src/php/php-5.2.10 -I/usr/local/src/php/php-5.2.10/dl -I/usr/local/src/php/php-5.2.10/main -I/usr/local/src/php/php-5.2.10/Zend -I/usr/local/src/php/php-5.2.10/include -I/usr/local/src/php/php-5.2.10/TSRM   -c -o php_mapscript_util.o php_mapscript_util.c
gcc -fPIC -g  -fPIC -Wall   -DUSE_PHP_REGEX -DCOMPILE_DL=1 -DPHP4 -DNEED_NONBLOCKING_STDERR      -DUSE_WMS_LYR -DUSE_WFS_LYR -DUSE_SOS_SVR -DUSE_LIBXML2 -DUSE_CURL -DUSE_WCS_SVR -DUSE_WFS_SVR -DUSE_WMS_SVR -DUSE_MING_FLASH   -DUSE_POSTGIS  -DUSE_GDAL -DUSE_OGR -DUSE_GEOS   -DUSE_PROJ -DUSE_EPPL  -DUSE_AGG   -DUSE_PDF  -DUSE_GD_GIF -DUSE_GD_PNG -DUSE_GD_JPEG -DUSE_GD_WBMP -DUSE_GD_FT -DGD_HAS_FTEX_XSHOW -DGD_HAS_GDIMAGEGIFPTR -DGD_HAS_GETBITMAPFONTS -DUSE_ICONV -DUSE_ZLIB  -I/home/tkralidi/foss4g/mapserver/svn/trunk/mapserver    -I/usr/include/libxml2  -I/usr/local/include   -I/usr/include  -I/usr/local/include -I/usr/local/include  -I/usr/local/include  -I/usr/include/freetype2   -I/usr/local/include -I/usr/include  -I/usr/include  -I/usr/local/src/php/php-5.2.10 -I/usr/local/src/php/php-5.2.10/dl -I/usr/local/src/php/php-5.2.10/main -I/usr/local/src/php/php-5.2.10/Zend -I/usr/local/src/php/php-5.2.10/include -I/usr/local/src/php/php-5.2.10/TSRM   -c -o php_mapscript.o php_mapscript.c
gcc -fPIC -g  -fPIC -Wall   -DUSE_PHP_REGEX -DCOMPILE_DL=1 -DPHP4 -DNEED_NONBLOCKING_STDERR      -DUSE_WMS_LYR -DUSE_WFS_LYR -DUSE_SOS_SVR -DUSE_LIBXML2 -DUSE_CURL -DUSE_WCS_SVR -DUSE_WFS_SVR -DUSE_WMS_SVR -DUSE_MING_FLASH   -DUSE_POSTGIS  -DUSE_GDAL -DUSE_OGR -DUSE_GEOS   -DUSE_PROJ -DUSE_EPPL  -DUSE_AGG   -DUSE_PDF  -DUSE_GD_GIF -DUSE_GD_PNG -DUSE_GD_JPEG -DUSE_GD_WBMP -DUSE_GD_FT -DGD_HAS_FTEX_XSHOW -DGD_HAS_GDIMAGEGIFPTR -DGD_HAS_GETBITMAPFONTS -DUSE_ICONV -DUSE_ZLIB  -I/home/tkralidi/foss4g/mapserver/svn/trunk/mapserver    -I/usr/include/libxml2  -I/usr/local/include   -I/usr/include  -I/usr/local/include -I/usr/local/include  -I/usr/local/include  -I/usr/include/freetype2   -I/usr/local/include -I/usr/include  -I/usr/include  -I/usr/local/src/php/php-5.2.10 -I/usr/local/src/php/php-5.2.10/dl -I/usr/local/src/php/php-5.2.10/main -I/usr/local/src/php/php-5.2.10/Zend -I/usr/local/src/php/php-5.2.10/include -I/usr/local/src/php/php-5.2.10/TSRM   -c -o mapscript_i.o mapscript_i.c
mapscript_i.c: In function 'layerObj_whichShapes':
mapscript_i.c:407: error: too many arguments to function 'msLayerWhichItems'
make[1]: *** [mapscript_i.o] Error 1
make[1]: Leaving directory `/home/tkralidi/foss4g/mapserver/svn/trunk/mapserver/mapscript/php3'
[devgeo:/home/tkralidi/foss4g/mapserver/svn/trunk/mapserver]$ ./mapserv

[sdlime]

I spaced MapScript Alan was fixing PHP and I'll take care of Swig...

Steve

[sdlime]

Added hooks for msLayerResultsGetShape() at r9200. At least for now all drivers just call their own msLayerGetShape() equivalent.

Steve

[sdlime]

Jim/Paul: If you guys could provide guidance on how to adapt the SDO and PostGIS work from the single pass sandbox to the RFC 52 scheme I'd appreciate it. The main difference being that msLayerGetShape() lives on "as is" and we're adding a msLayerResultsGetShape() instead (which simply wraps msLayerGetShape() for many drivers). For PostGIS I think this means 1) changing the index value written to a shapeObj index attribute to reference the row in a result set rather than some primary integer key and 2) adding a PostGIS implementation of msLayerResultsGetShape(). The SDO implementation was more involved...

Steve

[jimk]

Steve: This is from memory, I'll have to look at this more closely later. The OCI/SDO version needed to change some code to use scrollable cursors. Along with this the orig. code used one statement handle for all queries and I changed it so that self contained functions (basically anything but WhichShape/NextShape/ResultGetShape) used their own statement handle so they wouldn't invalidate the cursor if called after WhichShapes. With this change, I'd expect GetShape to live on as is in trunk except it would need to generate its own statement handle. WhichShape, NextShape, ResultGetShape would be as they are in the single pass branch. I will have to verify that the old GetShape doesn't try to cache results which could interfere with NextShape/ResultGetShape. The single pass implementation of ResultGetShape is very closely related to the NextShape implementation using scrollable cursors to go back to a previous shape and resetting the cache if necessary.

Jim K

[pramsey]

I'll have a go, Steve, I assume trunk now has all the core infrastructure in place?

[pramsey]

Committed the PostGIS changes at r9236, but it looks like the result set get shape isn't being called by the core code at this point.

[sdlime]

Thanks Paul. The calls to msResultNextShape() are sitting in my revisions on my dev box. I still have a few places to tweak but I should be able to commit by the end of the week. -Steve

[sdlime]

Replying to pramsey:

Committed the PostGIS changes at r9236, but it looks like the result set get shape isn't being called by the core code at this point.

Paul: I'm getting a "msPostGISLayerResultsGetShape(): General error message. PostgreSQL result set is not ready." error. Any hint or should I commit things "as is", could also supply a patch.

Steve

[pramsey]

Take the new one-pass function out of the postgis vtable so you can commit your changes, and I'll come back around and try and make things work now that the core machinery is hooked up.

[sdlime]

Core plumbing is committed in r9257. The two template methods, WFS/GML and WMS use the msLayerResultsGetShape() function. There are still outstanding issues:

• query maps: not all TYPEs work as yet because they sometimes drawing a layer normally before highlighting selected features. That won't work in this case. I think we can make a copy of the layer and draw the copy instead if necessary but I haven't tested that.

• there are a couple of spots in the OCG filter and SOS support that rely on msLayerGetShape() that will need visiting. I'll contact Tom and Assefa separately.

Steve

[sdlime]

Just an update... I'm working on adding the queryObj logic so save/load operations will work. Should finish in the next day or so. This shouldn't affect any of the driver changes that need to be made. The outstanding issues noted above are still present although I don't believe the OGC support will be a problem.

Steve

[jimk]

Updated Oracle Spatial driver to use new query code. There is more cleanup and testing that needs to be done, but I wanted to get this posted. Basic changes from trunk are: separate out statement handle, result buffers from the oracle connection handles so that a separate statement handle can be allocated for WhichShapes/NextShape/ResultGetShape from the rest of the code to prevent OCI calls in the rest of the code from clobbering the cursor/buffer created in WhichShapes. Change WhichShapes to create a scrollable cursor for use in ResultGetShape. Merge in ResultGetShape from the GetShape single pass branch.

Also changed msDrawVectorLayer to not close the layer when the layer has a resultscache defined because it was closing the layer before the template results were being processed.

[sdlime]

Crap, some changes by Alan to maporaclespatial.c yesterday make the patch application fail badly. Any chance you can easily regenerate?

Steve

[jimk]

Replying to sdlime:

Crap, some changes by Alan to maporaclespatial.c yesterday make the patch application fail badly. Any chance you can easily regenerate?

Steve

Not sure what you want me to do... It looks like the r9298 changes added some 4D code to maporaclespatial.c but it also looks like it created a number of regressions from at least as far back as the code sprint. I am weary of putting all the unrelated changes into one commit/patch.

[jimk]

correction r9295.

[sdlime]

Is there a logical grouping of changes that could be made (or proposed) from your perspective? Perhaps this could happen on the dev list so Alan and the other Oracle power users could chime in. You were proposing changes beyond those simply necessary to support random access to a SQL result set (as opposed to msLayerGetShape()), correct?

Steve

[jimk]

Patch updated to work with changes in r9295 (for 4D support).

[sdlime]

Ok, thanks for the patch Jim. I omitted the change to mapdraw.c. That code will get fixed in another manner. I applied the patch to maporaclespatial.c and committed to trunk (r9302). Probably worth some testing... ;-)

I'm curious if you see the performance increases you did in the sandbox. Just avoid query maps in testing and you should be fine.

Steve

[tomkralidis]

FYI I get the following when building with trunk:

maporaclespatial.c: In function 'msOracleSpatialLayerResultGetShapeVT':
maporaclespatial.c:3056: warning: implicit declaration of function 'msOracleSpatialLayerResultGetShape'
maporaclespatial.c: At top level:

[jimk]

Replying to sdlime:

Ok, thanks for the patch Jim. I omitted the change to mapdraw.c. That code will get fixed in another manner. I applied the patch to maporaclespatial.c and committed to trunk (r9302). Probably worth some testing... ;-)

I'm curious if you see the performance increases you did in the sandbox. Just avoid query maps in testing and you should be fine.

Steve

Steve: The new way is ~ 8 seconds, Mapserver 5.2 takes 1min 15sec for the same "nquery" request.

Tom: I have a patch that cleans this and a few other misc debug lines up.

[jimk]

The new one is returning less rows though so I need to look into it more... it could be the code or the test environment at this point.

[pramsey]

PostGIS should work now, as of r9311.

[jimk]

An update. I fixed a bug in my code in NextShape. I also found another case of the layer being closed by a draw function before the results are processed for the template.

And finally I found a problem where MapServer isn't handling some geometries returned by Oracle (a geometry with the WKT of "COMPOUNDCURVE ((488081.955 4975578.675, 488082.045 4975580.935), CIRCULARSTRING (488082.045 4975580.935, 488083.836 4975585.012, 488087.922 4975586.782), (488087.922 4975586.782, 488089.068 4975586.822, 488133.812 4975586.208))"... this creates symptoms of displaying the curve as a closed polygon to crashing in template processing (in GEOS) (using [shpxy buffer=]) and omitting the geometry from the WFS output... this last one should probably be a new bug.

[tomkralidis]

FYI I can't get svn trunk WFS Server against PostGIS working. Not sure what changed, but here are some tests, which worked before (say a month ago):

Underlying layer type is PostGIS, wfs_featureid is set and valid.

GetFeature, no filter (works)

http://devgeo.cciw.ca/cgi-bin/mapserv/owscat?version=1.1.0&service=WFS&request=GetFeature&typename=service_endpoints

GetFeature, simple filter (works)

http://devgeo.cciw.ca/cgi-bin/mapserv/owscat?service=WFS&version=1.0.0&request=GetFeature&typename=service_endpoints&filter=<Filter><PropertyIsEqualTo><PropertyName>lang</PropertyName><Literal>it-IT</Literal></PropertyIsEqualTo></Filter>

GetFeature, complex filter (does not work)

http://devgeo.cciw.ca/cgi-bin/mapserv/owscat?version=1.1.0&service=WFS&request=GetFeature&typename=service_endpoints&filter=<Filter><And><BBOX><PropertyName>NAME</PropertyName><Box srsName='EPSG:4326'><coordinates>-141,42 -52,84</coordinates></Box></BBOX><PropertyIsEqualTo><PropertyName>service_type</PropertyName><Literal>OGC:WMS</Literal></PropertyIsEqualTo><Or><Or><PropertyIsLike wildCard='*' singleChar='.' escape='!' matchCase='false'><PropertyName>title</PropertyName><Literal>*birds*</Literal></PropertyIsLike><PropertyIsLike wildCard='*' singleChar='.' escape='!' matchCase='false'><PropertyName>abstract</PropertyName><Literal>*birds*</Literal></PropertyIsLike></Or><PropertyIsLike wildCard='*' singleChar='.' escape='!' matchCase='false'><PropertyName>keywords</PropertyName><Literal>*birds*</Literal></PropertyIsLike></Or></And></Filter>

[assefa]

looking into this... I could not get a shape query working either (with a shape file in my case)

[tomkralidis]

Here's the simplest filter case I could run that produces the error:

http://localhost/cgi-bin/mapserv/owscat?version=1.1.0&service=WFS&request=GetFeature&typename=service_endpoints&filter=<Filter><PropertyIsLike wildCard='*' singleChar='.' escape='-' matchCase='false'><PropertyName>keywords</PropertyName><Literal>*birds*</Literal></PropertyIsLike></Filter>

[assefa]

committed mapogcfilter.c (r9313) and it seems to fix my test case. Can you please give it a try?

[assefa]

hold on on tests .. I still have an issue with other type of filters

[tomkralidis]

Replying to assefa:

committed mapogcfilter.c (r9313) and it seems to fix my test case. Can you please give it a try?

Assefa: I tested this. When I run

http://devgeo.cciw.ca/cgi-bin/mapserv/owscat?version=1.1.0&service=WFS&request=GetFeature&typename=service_endpoints&filter=<Filter><And><BBOX><PropertyName>NAME</PropertyName><Box srsName='EPSG:4326'><coordinates>-141,42 -52,84</coordinates></Box></BBOX><PropertyIsEqualTo><PropertyName>service_type</PropertyName><Literal>OGC:WMS</Literal></PropertyIsEqualTo><Or><Or><PropertyIsLike wildCard='*' singleChar='.' escape='!' matchCase='false'><PropertyName>title</PropertyName><Literal>*birds*</Literal></PropertyIsLike><PropertyIsLike wildCard='*' singleChar='.' escape='!' matchCase='false'><PropertyName>abstract</PropertyName><Literal>*birds*</Literal></PropertyIsLike></Or><PropertyIsLike wildCard='*' singleChar='.' escape='!' matchCase='false'><PropertyName>keywords</PropertyName><Literal>*birds*</Literal></PropertyIsLike></Or></And></Filter>

or

http://localhost/cgi-bin/mapserv/owscat?version=1.1.0&service=WFS&request=GetFeature&typename=service_endpoints&filter=<Filter><PropertyIsLike wildCard='*' singleChar='.' escape='-' matchCase='false'><PropertyName>keywords</PropertyName><Literal>*birds*</Literal></PropertyIsLike></Filter>

I get the following:

{{{<?xml version='1.0' encoding="ISO-8859-1" ?> <wfs:FeatureCollection

xmlns:owscat="http://www.ec.gc.ca/owscat" xmlns:gml="http://www.opengis.net/gml" xmlns:wfs="http://www.opengis.net/wfs" xmlns:ogc="http://www.opengis.net/ogc" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.ec.gc.ca/owscat ​http://devgeo.cciw.ca/cgi-bin/mapserv/owscat?SERVICE=WFS&amp;VERSION=1.1.0&amp;REQUEST=DescribeFeatureType&amp;TYPENAME=service_endpoints&amp;OUTPUTFORMAT=text/xml; subtype=gml/3.1.1 ​http://www.opengis.net/wfs ​http://schemas.opengis.net/wfs/1.1.0/wfs.xsd" numberOfFeatures="3">

<gml:boundedBy>

<gml:Envelope srsName="EPSG:4326">

<gml:lowerCorner>-1.000000 -1.000000</gml:lowerCorner> <gml:upperCorner>-1.000000 -1.000000</gml:upperCorner>

</gml:Envelope>

</gml:boundedBy>

<!-- WARNING: FeatureId item 'service_id' not found in typename 'service_endpoints'. -->

Content-type: text/xml

<?xml version="1.0" encoding="ISO-8859-1"?> <ows:ExceptionReport xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:ows="http://www.opengis.net/ows" version="1.1.0" language="en-US" xsi:schemaLocation="http://www.opengis.net/ows http://schemas.opengis.net/ows/1.0.0/owsExceptionReport.xsd">

<ows:Exception exceptionCode="NoApplicableCode" locator="mapserv">

<ows:ExceptionText>msGMLWriteWFSQuery(): WFS server error. GML transformation error

msPostGISLayerResultsGetShape(): General error message. Got request larger than result set.</ows:ExceptionText>

</ows:Exception>

</ows:ExceptionReport>}}}

[jimk]

Bugfix

[jimk]

FYI: something between 9303 and 9313 is causing double frees and modifying of pointers after they are free'ed.

[jimk]

Replying to jimk:

FYI: something between 9303 and 9313 is causing double frees and modifying of pointers after they are free'ed.

It problem started in r9307.

[sdlime]

Ok, been gone for a couple of days, sorry for the delay.

1) Assefa, the PostGIS driver still needs work. Paul made an initial pass at it but the overall plumbing was in place so he couldn't test. Try testing your stuff against a shapefile for now.

2) Jim, I take it I need to apply both patches you supplied or does the last one suplant the first? Please advise.

We'll get there... ;-)

Steve

[pramsey]

FYI, as of r r9311, PostGIS should work OK.

[assefa]

did another update to mapogcfilter.c (r9314). Initial tests doing few queries on a postgis were successful. Tom, if possible I would like to do more tests using the same data set as yours. Let me know.

[jimk]

2) Jim, I take it I need to apply both patches you supplied or does the last one suplant the first? Please advise.

They are full patches against the revision indicated in filename. Keep in mind I still have to go back to r9306 to avoid core dumps. Something changed in mapserver in r9307 that is causing memory management issues. (I haven't been able to track down what change is causing the problem, but it is being reported as a double free's and modifying free'd objects by the malloc debugger)

So the latest would apply to r9313 and the svn update -r 9306 if you want it to actually work.

Sorry for all the confusion.

[tomkralidis]

Replying to assefa:

did another update to mapogcfilter.c (r9314). Initial tests doing few queries on a postgis were successful. Tom, if possible I would like to do more tests using the same data set as yours. Let me know.

We're getting closer. I am getting results back (see the URLs in comment:30 now). Looks like the filters are working. Good work Assefa!

However, the table I'm querying has 123 records (select count(*) from table). Doing a GetFeature request (with no filter) against the layer (which should return everything, which is 123 records) returns a GML document with 112 records. FYI I don't have max_features turned on in the mapfile.

[sdlime]

Jim/Paul: What will the PostGIS and Oracle drivers do if msXXXResultsGetShape() is called and there is no result set at the database level? Could it try msXXXGetShape() in that case?

Here's the reason. We have a function called msQueryByIndex() that allows fetching a shape using shape and tile indexes. Basically it's a query wrapper for msLayerGetShape() and will continue to work that way. Both PostGIS and Oracle should still support a msXXXLayerGetShape function that uses the unique key identified in the DATA statement.

Does that make sense?

Steve

[sdlime]

Replying to jimk:

2) Jim, I take it I need to apply both patches you supplied or does the last one suplant the first? Please advise.

They are full patches against the revision indicated in filename. Keep in mind I still have to go back to r9306 to avoid core dumps. Something changed in mapserver in r9307 that is causing memory management issues. (I haven't been able to track down what change is causing the problem, but it is being reported as a double free's and modifying free'd objects by the malloc debugger)

So the latest would apply to r9313 and the svn update -r 9306 if you want it to actually work.

Sorry for all the confusion.

Still confused, but I think you're saying apply the latest patch, but stuff is busted because of a change in 9307 so good luck running anything later...

Steve

[jimk]

Replying to sdlime:

Still confused, but I think you're saying apply the latest patch, but stuff is busted because of a change in 9307 so good luck running anything later...

Steve

You got it.

[jimk]

Replying to sdlime:

Jim/Paul: What will the PostGIS and Oracle drivers do if msXXXResultsGetShape() is called and there is no result set at the database level? Could it try msXXXGetShape() in that case?

Here's the reason. We have a function called msQueryByIndex() that allows fetching a shape using shape and tile indexes. Basically it's a query wrapper for msLayerGetShape() and will continue to work that way. Both PostGIS and Oracle should still support a msXXXLayerGetShape function that uses the unique key identified in the DATA statement.

Does that make sense?

Steve

I was thinking about this and the functionality was in one version of my patch (it is probably commented out in the latest.) The problem I have is I need to know if a shape/tileindex is a real shape index (primary key) or a resultset index (index into a cursor). My solution was to use shape->index for the pkey and shape->tileindex for the resultset index. This is mostly ok because tileindex isn't used by the oracle spatial driver. The main benefit is that shape->index is always the primary key for mapscript users who expect this behavior. The problem is that tileindex is an int32 where shape index is an int64, so bad things will happen if a query returns more than 2^{32-1 rows.}

The way I had it setup was that if LayerResultGetShape did not find an open layer with a valid result set it would just call return LayerGetShape(..., index) to fall back to the old behavior.

In the case of msQueryByIndex, why wouldn't it just always call msXXXLayerGetShape? msOracleSpatialGetShape is always valid resultset or not, and does not interfere with an existing result set if one exists. Also the ...ResultGetShape is expecting an index into a cursor not a primary key so how would the user know what index value to pass into msQueryByShape anyway?

So summary... yes it can work, but I'm not sure why we'd want it to.

[tomkralidis]

Replying to tomkralidis:

Replying to assefa:

did another update to mapogcfilter.c (r9314). Initial tests doing few queries on a postgis were successful. Tom, if possible I would like to do more tests using the same data set as yours. Let me know.

We're getting closer. I am getting results back (see the URLs in comment:30 now). Looks like the filters are working. Good work Assefa!

However, the table I'm querying has 123 records (select count(*) from table). Doing a GetFeature request (with no filter) against the layer (which should return everything, which is 123 records) returns a GML document with 112 records. FYI I don't have max_features turned on in the mapfile.

Paul: as per our IRC discussion, not sure if you got my testcase for this issue. I can send it to you offline.

[assefa]

Tom, I tried quickly to see if I can reproduce the inconsistency on the number of features but could not. Any chance the extents could play a role in this (map->extent and/or ows_extent on the layer ?). if possible you can also send me your test cases/data. Thanks

[tomkralidis]

Replying to assefa:

Tom, I tried quickly to see if I can reproduce the inconsistency on the number of features but could not. Any chance the extents could play a role in this (map->extent and/or ows_extent on the layer ?). if possible you can also send me your test cases/data. Thanks

Ah. Thanks Assefa.

Mea culpa. I had ows_extent set on that layer. Unsetting it returns all 123 records.

Paul: ignore the testing on this one.

[sdlime]

One long ticket... I just committed the support for the queryObj encapsulation (r9323). This requires that you set the map->query structure appropriately before calling the various query methods. I also added a msExecuteQuery() function that calls the right low level function based on map->query. The CGI uses that instead of calling the low level stuff which simplifies the code a bit. I suspect the OGC filter code could be simplified a bit with this change. (The raster query code is untouched at this point and needs testing.)

I updated all the OGC service code, PHP/MapScript and Swig/MapScript. I didn't test the PHP compile but it's a straight copy of the Swig stuff which does compile fine. The OGC code compiles too. There were no underlying logic changes to the various query code so everything *should* work ok. I did a bunch of non-OGC testing with shapefiles and results were as expected.

The new save/load code is in place. I added a magic string to the query file loading out of paranoia.

I also exposed the resultsGetShape() method in Swig/MapScript. Recall it's the new layer level function for accessing result sets. I did not add it to PHP/MapScript. The one decision to make is regarding argument order. Right now it matches getFeature() which passes shapeindex then tileindex. The getShape() method passes tileindex then shapeindex. Any preference? (I'd like to fix syntax things like this in 6.0)...

Tom, I noticed the mapogcsos.c code still is calling msLayerOpen() etc... after a query is done. Not necessary anymore. The layer is left open once the query is done so you should just need to start accessing results using msLayerResultsGetShape().

Let me know if you find issues. There still is a bug in the query map drawing code but it's late and I'm tired.

Steve

[tomkralidis]

Replying to sdlime:

One long ticket... I just committed the support for the queryObj encapsulation (r9323). This requires that you set the map->query structure appropriately before calling the various query methods. I also added a msExecuteQuery() function that calls the right low level function based on map->query. The CGI uses that instead of calling the low level stuff which simplifies the code a bit. I suspect the OGC filter code could be simplified a bit with this change. (The raster query code is untouched at this point and needs testing.)

I updated all the OGC service code, PHP/MapScript and Swig/MapScript. I didn't test the PHP compile but it's a straight copy of the Swig stuff which does compile fine. The OGC code compiles too. There were no underlying logic changes to the various query code so everything *should* work ok. I did a bunch of non-OGC testing with shapefiles and results were as expected.

The new save/load code is in place. I added a magic string to the query file loading out of paranoia.

I also exposed the resultsGetShape() method in Swig/MapScript. Recall it's the new layer level function for accessing result sets. I did not add it to PHP/MapScript. The one decision to make is regarding argument order. Right now it matches getFeature() which passes shapeindex then tileindex. The getShape() method passes tileindex then shapeindex. Any preference? (I'd like to fix syntax things like this in 6.0)...

Tom, I noticed the mapogcsos.c code still is calling msLayerOpen() etc... after a query is done. Not necessary anymore. The layer is left open once the query is done so you should just need to start accessing results using msLayerResultsGetShape().

Let me know if you find issues. There still is a bug in the query map drawing code but it's late and I'm tired.

Steve

Steve: I get the following when trying to build:

mapscript_i.c: In function 'layerObj_queryByShape':
mapscript_i.c:598: error: too many arguments to function 'msQueryByShape'
make[1]: *** [mapscript_i.o] Error 1

The msautotest/wxs tests look like they run fine (they don't all pass, but these issues are not related to this ticket).

Will check on the mapogcsos.c code with Assefa.

[sdlime]

Tom: I fixed that last call to msQueryByShape(). Should compile ok.

Steve

[jimk]

Fixed type bugs (long->ub4), fixed handeling of compound geometries (type 42) always being forced to MS_SHAPE_POLYGON

[sdlime]

Jimk, applied the last patch in r9352. -Steve

[sdlime]

Fixed query maps (I believe) in r9357. I make a copy of the queried layer (sans results) and draw that ahead of the results for HILITE and NORMAL query map styles. I've only tested with shapefiles although it should be ok with all layer types.

Steve

[jimk]

This looks like it will work.

However, msOracleSpatialLayerResultGetShape is failing because too much is getting cleaned up in msOracleSpatialLayerClose and that is disconnecting other layers from the DB.

Solution is to move the call to OCICacheFree from when the layer is closed to when the connection is closed (via the connection pool).

Patch attached.

[sdlime]

Jim, applied the most recent patch (r9358)... Thanks!

Steve

[dmorissette]

layer.resultsGetShape() added to PHP MapScript in r9359.

Also created ticket #3140 about documenting the new method

[aboudreault]

The changeset r9358 is the cause of the oracle memory leak in fastcgi. See related bug: ​http://trac.osgeo.org/mapserver/ticket/3187

[warmerdam]

I have implemented for OGR in trunk (post 5.6) (r9640).

[sdlime]

Replying to warmerdam:

I have implemented for OGR in trunk (post 5.6) (r9640).

Thanks for working on this Frank!

Is this something that could be added to 5.6.1? It's in that grey area between bug fix and new feature. Personally I'd error on the side of completeness in 5.6 and backport.

Steve

[warmerdam]

Steve,

I'm somewhat conflicted on the matter. I think it would be a valuable addition in 5.6.1 but it does run the risk of breaking some applications that worked in 5.6.0.

[jimk]

My two cents is that the mapscript API changed in 5.6 (GetShape -> GetResultShape) so that any script that is compatible with 5.6.0 will continue to work. (Scripts written for 5.4 and earlier may need to be modified.)

An incompatible script might still work in 5.6.0 depending on the implementation of the underlying driver, but that is not the intent for how things should be called under 5.6.x... so even if the script does work, it is still broken (say if the datasource changes). It just doesn't know it yet.

[tamas]

Replying to sdlime:

Fixed query maps (I believe) in r9357. I make a copy of the queried layer (sans results) and draw that ahead of the results for HILITE and NORMAL query map styles. I've only tested with shapefiles although it should be ok with all layer types.

Steve

I looks appropriate to create a new layer for drawing but adding that layer to the map permanently triggers at least the following issues:

1. It seems we make an assumption that the mapObj is destroyed after each drawQuery, however that's not the case with many of the mapscript applications. This approach would bring in extra unexpected layers for the users which are difficult to interpret.

2. msCopyLayer doesn't seem to work correctly when the destination have already been initialized (see the implementation of msCopyProjection, insertFeatureList, copying the classes etc). This would at least result in memory leaks and assertion failures.

I've created a patch (mapdraw.c.patch) to implement this kind of drawing without requiring to add the layer to the map.

[tamas]

mapdraw.c.patch applied in r9663 and r9664