Ticket to track implementation of RFC 65: One-pass Query Improvements

[sdlime]

This ticket exists to track changes associated with RFC 65. For more information on that ticket see:

http://mapserver.org/development/rfc/ms-rfc-65.html

Steve

[sdlime]

Since we can't be positive that we can re-purpose a tile index I think it's safest to simply add a new index (resultindex) to shapeObj and resultCacheMemberObj structures. That index will carry an index that can be used within a session result set. It cannot be cached. Cached result sets can only carry the shape/tile indexes. See r10828...

Steve

[sdlime]

Whoops, that's 10826... Steve

[sdlime]

Added the code to allow for multiple styles of query results serialization in r10923 and r10924. In the end there's a new boolean to saveQuery that triggers to old-style query file creation (e.g. shape/tile indexes). Here's a little perl script that shows how to save a query:

#!/usr/bin/perl

use mapscript;

$map = new mapscript::mapObj('test.map');
$layer = $map->getLayerByName('counties');

$layer->queryByRect($map, $map->{extent});

print $layer->getNumResults() ."\n";

$map->saveQuery('output_params.qy');
$map->saveQuery('output_results.qy', $mapscript::MS_TRUE);

Loading a query is unchanged. Each type of query file has a different magic string so the type is automatically detected. Here's a little load test script:

#!/usr/bin/perl

use mapscript;

$map = new mapscript::mapObj('test.map');
$layer = $map->getLayerByName('counties');

$map->loadQuery('output_results.qy');
print $layer->getNumResults() ."\n";

$map->freeQuery();
print $layer->getNumResults() ."\n";

$map->loadQuery('output_params.qy');
print $layer->getNumResults() ."\n";

Now onto the getShape() changes.

Steve

[sdlime]

Waiting on word from Assefa regarding removing a few unnecessary functions that I would otherwise have to patch because of these changes. Otherwise I'm ready to go. The msLayerGetShape() function has been changed to take a resultObj (formerly resultCacheMemberObj) instead of individual indexes. In perl MapScript query processing is quite simple:

#!/usr/bin/perl                                                                                                                              

use mapscript;

$map = new mapscript::mapObj('test.map');
$layer = $map->getLayerByName('counties');

$layer->queryByRect($map, $map->{extent}); # layer is still open                                                                             

for($i=0; $i<$layer->getNumResults(); $i++) {
    $shape = $layer->getShape($layer->getResult($i));
    print "$i: ". $shape->getValue(5) ."\n";
}

$layer->close();

The todo's include:

• making sure OGR, PostGIS and Oracle Spatial work. I changed enough to get things to compile but that is all (I left TODO comments where I think changes need to be made).
  • merge resultsGetShape and getShape
  • make sure correct indexes are set in nextShape
• update PHP/MapScript
  • change layer getShape to mirror SWIG version
  • account for resultMemberCacheObj => resultObj change
• write tests
• document

Steve

[sdlime]

CC'ing Tamas since the MS SQL Server 2008 driver will likely need a few tweaks. I have no way to test any changes I'd make so it's probably best left alone.

Steve

[assefa]

Unused functions are removed (r10932)

[sdlime]

Ok, I committed all the changes I made last night in r10933. I touched a lot of files but the changes were generally mild.

Again, PHP/MapScript will not build without some updates (see layer.i and resultcache.i), nor will the MS SQL Server 2008 driver.

Steve

[sdlime]

Replying to assefa:

Unused functions are removed (r10932)

I also think we can remove the usegetshape member from the resultCacheObj struct. Ok?

Steve

[dmorissette]

Alan: can you please take care of the updates to PHP MapScript?

[rouault]

trunk build is broken when building with the following conf :

./configure --with-gdal=/home/even/gdal/svn/trunk/gdal/apps/gdal-config --with-ogr=/home/even/gdal/svn/trunk/gdal/apps/gdal-config --with-wfs --with-proj --with-threads --with-xml2-config=/usr/bin/xml2-config --with-geos --with-curl-config=/usr/bin/curl-config --with-wmsclient --with-wfsclient --with-postgis --with-cairo --with-png=/usr --with-jpg=/usr --with-experimental-png --with-wcs --with-wcsclient --with-sos

I need the following patch to make it *compile*, but linking fails because msLayerResultsGetShape has been commented

Index: mapgml.c
===================================================================
--- mapgml.c	(révision 10934)
+++ mapgml.c	(copie de travail)
@@ -1457,9 +1457,9 @@
         }
 
         if(lp->resultcache->usegetshape)
-           status = msLayerGetShape(lp, &shape, lp->resultcache->results[j].tileindex, lp->resultcache->results[j].shapeindex);
+           status = msLayerGetShape(lp, &shape, &(lp->resultcache->results[j]));
         else
-          status = msLayerResultsGetShape(lp, &shape, lp->resultcache->results[j].tileindex, lp->resultcache->results[j].shapeindex);        
+          status = msLayerResultsGetShape(lp, &shape, &(lp->resultcache->results[j]));        
         if(status != MS_SUCCESS) 
             return(status);
 
Index: mapwfs.c
===================================================================
--- mapwfs.c	(révision 10934)
+++ mapwfs.c	(copie de travail)
@@ -2567,7 +2567,7 @@
                 {
                     memmove( lp->resultcache->results + 0,
                              lp->resultcache->results + to_skip, 
-                             sizeof(resultCacheMemberObj) * (lp->resultcache->numresults - to_skip) );
+                             sizeof(resultObj) * (lp->resultcache->numresults - to_skip) );
                     lp->resultcache->numresults -= to_skip;
                     to_skip = 0;
                 }
Index: mapserver.h
===================================================================
--- mapserver.h	(révision 10934)
+++ mapserver.h	(copie de travail)
@@ -2071,7 +2071,7 @@
 int msOGRLayerNextShape(layerObj *layer, shapeObj *shape);
 int msOGRLayerGetItems(layerObj *layer);
 void msOGRLayerFreeItemInfo(layerObj *layer);
-int msOGRLayerGetShape(layerObj *layer, shapeObj *shape, int tile, long record);
+int msOGRLayerGetShape(layerObj *layer, shapeObj *shape, resultObj *record);
 int msOGRLayerResultGetShape(layerObj *layer, shapeObj *shape, int tile, long record);
 int msOGRLayerGetExtent(layerObj *layer, rectObj *extent);

[sdlime]

Nuts, I'm sorry I didn't catch these before. Fixed these in r10935. The msLayerResultGetShape() function is gone so the change to mapgml.c was a little different than what's in the patch.

Trunk compiles and links for me at least. Note that OGR, PostGIS and Oracle Spatial queries are probably broken until a few more changes are made.

Steve

[rouault]

I have the following linking errors

./libmapserver.a(mapwfslayer.o): In function `msWFSLayerResultGetShape':
mapwfslayer.c:(.text+0x2df): undefined reference to `msOGRLayerResultGetShape'
./libmapserver.a(mapogcsos.o): In function `msSOSDescribeSensor':
mapogcsos.c:(.text+0x1572): undefined reference to `msLayerResultsGetShape'
./libmapserver.a(mapogcsos.o): In function `msSOSReturnMemberResult':
mapogcsos.c:(.text+0x186f): undefined reference to `msLayerResultsGetShape'
./libmapserver.a(mapogcsos.o): In function `msSOSAddMemberNode':
mapogcsos.c:(.text+0x2d79): undefined reference to `msLayerResultsGetShape'
./libmapserver.a(mapogcsos.o): In function `msSOSGetCapabilities':
mapogcsos.c:(.text+0x48f4): undefined reference to `msLayerResultsGetShape'
./libmapserver.a(mapogcsos.o): In function `msSOSGetObservation':
mapogcsos.c:(.text+0x75db): undefined reference to `msLayerResultsGetShape'

[sdlime]

Give it a try now, fixed those in r10936. -Steve

[rouault]

This works now. Thanks

[tamas]

Modified mapmssql2008.c to follow the recent changes in the query handling (r10943)

[aboudreault]

PHP/MapScript updated accordingly to the rfc 65. It is very similar to perl/mapscript:

$map = new mapObj("gmap75.map");
$l = $map->getLayerByName("popplace");
$l->queryByRect($map->extent);
for ($i=0; $i<$l->getNumResults();$i++){
    $s = $l->getShape($l->getResult($i));
    echo $s->getValue($l,"Name");
    echo "\n";
}

Documentation need to be updated.

[aboudreault]

forgot to mention the changeset: r10964.

[aboudreault]

Steve, just to confirm.... How would you call getShape() to get a single shape without executing anyt whichShape or queryBy* methods? You would create a resultObj manually, then set the shapeindex? (resultindex==-1)

[sdlime]

Replying to aboudreault:

Steve, just to confirm.... How would you call getShape() to get a single shape without executing anyt whichShape or queryBy* methods? You would create a resultObj manually, then set the shapeindex? (resultindex==-1)

Correct. We might need a resultObj class then (result.i in swiginc). Constructor would take a shapeindex, tileindex but no resultindex. Tileindex would default to -1.

Usage would be something like:

$layer->open();
my $shape = $layer->getShape(new mapscript::resultObj(5));

At least that's what I was thinking anyway. That's the most common use case I think.

Steve

[aboudreault]

Modified PHP/MapScript to be able to instantiate a resultObj in r10969. ie:

$layer->open();
$shape = $layer->getShape(new resultObj(491)); // 491 == shapeindex

Fixed OracleSpatial driver in 10970 and PostGIS driver in 10971.

[aboudreault]

enabling changeset links: Fixed OracleSpatial driver in r10970 and PostGIS driver in r10971.

[aboudreault]

I have added the ability to create a resultObj to do single getShape in SWIG/MapScript in 10972. However, the queryByRect doesn't seem to work using a postgis layer. The single getShape query works though. It works with an OracleSpatial layer. I tested with a python script:

from mapscript import *

szMapFile = "gmap75.map"
oMap = mapObj(szMapFile)

l = oMap.getLayerByName('popplace')
l.open()
s = l.getShape(resultObj(1));
print s.getValue(5)
l.close()
print oMap.extent
l.queryByRect(oMap, oMap.extent)
print l.getNumResults()
i=0
for i in range(l.getNumResults()):
    result = l.getResult(i)
    s = l.getShape(result)
    print s.getValue(5)

getNumResults() is always 0. The same script in PHP works well for Oracle and PostGIS. Could anyone test it with any mapscript binding?

[sdlime]

Hi Alan: I tested with perl:

#!/usr/bin/perl

use mapscript;

$map = new mapscript::mapObj('query_postgis.map') or die 'Unable to open mapfile.';
$l = $map->getLayerByName('bdry_counpy2') or die 'Layer not found.';

$l->open();
$s = $l->getShape(new mapscript::resultObj(1));
print $s->getValue(6) ."\n"; # county name
$l->close();

$rect = new mapscript::rectObj(420000,5120000,582000,5200000);
$l->queryByRect($map, $rect);
print $l->getNumResults() ."\n";
for($i=0; $i<$l->getNumResults(); $i++) {
    $r = $l->getResult($i);
    $s = $l->getShape($r);
    print $s->getValue(6) ."\n"; # county name 
}

This runs fine against a polygon PostGIS layer I have here. I also tested with the CGI (see msautotest/query) and things worked ok.

Did you do any performance tests with and without the resultindex? If not the easiest way would be using new and old query files. The new query file will always create a new result set and therefore should be much faster. The old query file will create a SQL statement per result for the DB drivers.

Steve

[aboudreault]

Steve, I'm not very familiar with query files. What do you mean by old query files? Using MS 5.6.5?

[sdlime]

If you look at the first of the changes for this ticket you'll see references to the two "styles" of query files.

The new-style (version 5.4+) just serializes a queries parameters to a file. The old-style (all other versions) serializes the feature index values to a file.

In 6.0 both are supported. Anyway, you can use query files of each style to test performance with Oracle, PostGIS, MSSQL Server and certain OGR sources. The new-style files create in one result set and getShape() operates against it (using resultindex). The old-style query files should be much slower with larger result sets because each getShape() results in an new SQL query (as there is no resultindex).

A test script might look like:

#!/usr/bin/perl

use mapscript;

$map = new mapscript::mapObj('test.map');
$layer = $map->getLayerByName('counties');

# do a query
$layer->queryByRect($map, $map->{extent});

# save the results to files
$map->saveQuery('output_params.qy');
$map->saveQuery('output_results.qy', $mapscript::MS_TRUE);

foreach $qf in ('output_params.qy', 'output_results.qy') {
  # start a timer

  # load query
  $map->loadQuery($qf);

  $layer->open(); # just in case
  for($i=0; $i<$layer->getNumResults(); $i++) {
    $shape = $layer->getShape($layer->getResult($i));
    # perhaps print something
  }

  # clear query
  $map->freeQuery();
 
  # stop timer, print results
}

# clean up

For certain drivers we should see large differences in timings between approaches. For others, like shapefiles, the difference should be negligible.

The old-style files were nice for certain types of operations since you muck with them. That's why I brought back that functionality.

Does this help?

Steve

[tamas]

I've found significant problems with regards to the OGR driver and the single pass implementation. It seems the current implementation doesn't follow the concept have been introduced in RFC 65, I attach a patch which should somewhat handle this confusion. It seems we also require to change the signature of LayerGetAutoStyle to make sure the resultindex is also passed through.

Another issue with this OGR implementation (which is not handled in the patch) is that when drawing the query map in MS_NORMAL or MS_HILITE the background layer messes up the spatial filter of the query layer bacause of the connection pooling.

[tamas]

Java example was fixed according to RFC 65 (r11079)

[tamas]

The mappool change was applied in r11041 OGR fix was applied in r11042

[rouault]

Compiling with clang reveals the following warnings, that seem to be serious in case those functions are indeed called. Perhaps related to the implementation of this RFC.

mapmygis.c:1985:34: warning: incompatible pointer types assigning to 'int (*)(layerObj *, shapeObj *, resultObj *)' from 'int (layerObj *, shapeObj *, int, long)'
    layer->vtable->LayerGetShape = msMYGISLayerGetShapeVT;
                                 ^ ~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

mapgraticule.c:1027:36: warning: incompatible pointer types assigning to 'int (*)(mapObj *, layerObj *, classObj *, shapeObj *)' from 'int (mapObj *, layerObj *, classObj *, int, long)'
  layer->vtable->LayerGetAutoStyle = msGraticuleLayerGetAutoStyle;
                                   ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

[tamas]

Replying to rouault:

Compiling with clang reveals the following warnings, that seem to be serious in case those functions are indeed called. Perhaps related to the implementation of this RFC.

Corrected the issue of mapgraticule.c in r11123

mapmygis.c doesn't worth to deal with. It will be removed.