Skip to content

Commit

Permalink
[Filter] Fix slow filter BBOX evaluation with tileindex of shapefile (#…
Browse files Browse the repository at this point in the history
…5291)

On a layer that is a tileindex of shapefiles, a BBOX as WFS XML filter
used the slow path of common expression evaluation, which caused all
shapefiles to be opened, instead of just the ones intersecting the BBOX.
This commit tries to fetch a "top BBOX" filter and use it as the rect
passed to msLayerWhichShapes().
  • Loading branch information
rouault committed Jun 22, 2016
1 parent 83a51fc commit 9f1ef95
Show file tree
Hide file tree
Showing 15 changed files with 229 additions and 25 deletions.
103 changes: 87 additions & 16 deletions mapogcfilter.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
#include "mapows.h"
#include <ctype.h>

#if 0
static int FLTHasUniqueTopLevelDuringFilter(FilterEncodingNode *psFilterNode);
#endif

int FLTIsNumeric(const char *pszValue)
{
Expand Down Expand Up @@ -569,8 +571,6 @@ int FLTApplySimpleSQLFilter(FilterEncodingNode *psNode, mapObj *map, int iLayerI
/************************************************************************/
int FLTIsSimpleFilter(FilterEncodingNode *psNode)
{
return MS_FALSE;

if (FLTValidForBBoxFilter(psNode)) {
if (FLTNumberOfFilterType(psNode, "DWithin") == 0 &&
FLTNumberOfFilterType(psNode, "Intersect") == 0 &&
Expand Down Expand Up @@ -614,17 +614,43 @@ int FLTApplyFilterToLayer(FilterEncodingNode *psNode, mapObj *map, int iLayerInd
/************************************************************************/
int FLTLayerApplyCondSQLFilterToLayer(FilterEncodingNode *psNode, mapObj *map, int iLayerIndex)
{
/* ==================================================================== */
/* Check here to see if it is a simple filter and if that is */
/* the case, we are going to use the FILTER element on */
/* the layer. */
/* ==================================================================== */
layerObj* lp = GET_LAYER(map, iLayerIndex);
if (FLTIsSimpleFilter(psNode) && !(lp->connectiontype == MS_OGR && !FLTHasUniqueTopLevelDuringFilter(psNode)) ) {
return FLTApplySimpleSQLFilter(psNode, map, iLayerIndex);
return FLTLayerApplyPlainFilterToLayer(psNode, map, iLayerIndex);
}


/************************************************************************/
/* FLTGetTopBBOX */
/* */
/* Return the "top" BBOX if there's a unique one. */
/************************************************************************/
static int FLTGetTopBBOXInternal(FilterEncodingNode *psNode, FilterEncodingNode** ppsTopBBOX, int *pnCount)
{
if (psNode->pszValue && strcasecmp(psNode->pszValue, "BBOX") == 0) {
(*pnCount) ++;
if( *pnCount == 1 )
{
*ppsTopBBOX = psNode;
return TRUE;
}
*ppsTopBBOX = NULL;
return FALSE;
}
else if (psNode->pszValue && strcasecmp(psNode->pszValue, "AND") == 0) {
return FLTGetTopBBOXInternal(psNode->psLeftNode, ppsTopBBOX, pnCount) &&
FLTGetTopBBOXInternal(psNode->psRightNode, ppsTopBBOX, pnCount);
}
else
{
return TRUE;
}
}

return FLTLayerApplyPlainFilterToLayer(psNode, map, iLayerIndex);
static FilterEncodingNode* FLTGetTopBBOX(FilterEncodingNode *psNode)
{
int nCount = 0;
FilterEncodingNode* psTopBBOX = NULL;
FLTGetTopBBOXInternal(psNode, &psTopBBOX, &nCount);
return psTopBBOX;
}

/************************************************************************/
Expand All @@ -637,12 +663,56 @@ int FLTLayerApplyPlainFilterToLayer(FilterEncodingNode *psNode, mapObj *map,
{
char *pszExpression =NULL;
int status =MS_FALSE;
layerObj* lp = GET_LAYER(map, iLayerIndex);

pszExpression = FLTGetCommonExpression(psNode, GET_LAYER(map, iLayerIndex));
if(map->debug == MS_DEBUGLEVEL_VVV)
msDebug("FLTLayerApplyPlainFilterToLayer(): %s\n", pszExpression);
pszExpression = FLTGetCommonExpression(psNode, lp);
if (pszExpression) {
status = FLTApplyFilterToLayerCommonExpression(map, iLayerIndex, pszExpression);
FilterEncodingNode* psTopBBOX;
rectObj rect = map->extent;

psTopBBOX = FLTGetTopBBOX(psNode);
if( psTopBBOX )
{
int can_remove_expression = MS_TRUE;
const char* pszEPSG = FLTGetBBOX(psNode, &rect);
if(pszEPSG && map->projection.numargs > 0) {
projectionObj sProjTmp;
msInitProjection(&sProjTmp);
/* Use the non EPSG variant since axis swapping is done in FLTDoAxisSwappingIfNecessary */
if (msLoadProjectionString(&sProjTmp, pszEPSG) == 0) {
rectObj oldRect = rect;
msProjectRect(&sProjTmp, &map->projection, &rect);
/* If reprojection is involved, do not remove the expression */
if( rect.minx != oldRect.minx ||
rect.miny != oldRect.miny ||
rect.maxx != oldRect.maxx ||
rect.maxy != oldRect.maxy )
{
can_remove_expression = MS_FALSE;
}
}
msFreeProjection(&sProjTmp);
}

/* Small optimization: if the query is just a BBOX, then do a */
/* msQueryByRect() */
if( psTopBBOX == psNode && can_remove_expression )
{
msFree(pszExpression);
pszExpression = NULL;
}
}

if(map->debug == MS_DEBUGLEVEL_VVV)
{
if( pszExpression )
msDebug("FLTLayerApplyPlainFilterToLayer(): %s, rect=%f,%f,%f,%f\n", pszExpression, rect.minx, rect.miny, rect.maxx, rect.maxy);
else
msDebug("FLTLayerApplyPlainFilterToLayer(): rect=%f,%f,%f,%f\n", rect.minx, rect.miny, rect.maxx, rect.maxy);
}

status = FLTApplyFilterToLayerCommonExpressionWithRect(map, iLayerIndex,
pszExpression, rect);
msFree(pszExpression);
}

Expand Down Expand Up @@ -1784,6 +1854,7 @@ int FLTValidForBBoxFilter(FilterEncodingNode *psFilterNode)
return 0;
}

#if 0
static int FLTHasUniqueTopLevelDuringFilter(FilterEncodingNode *psFilterNode)
{
int nCount = 0;
Expand All @@ -1809,7 +1880,7 @@ static int FLTHasUniqueTopLevelDuringFilter(FilterEncodingNode *psFilterNode)

return 0;
}

#endif

int FLTIsLineFilter(FilterEncodingNode *psFilterNode)
{
Expand Down
1 change: 1 addition & 0 deletions mapogcfilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ int FLTCheckInvalidProperty(FilterEncodingNode *psFilterNode,
mapObj *map, int i);
FilterEncodingNode* FLTSimplify(FilterEncodingNode *psFilterNode,
int* pnEvaluation);
int FLTApplyFilterToLayerCommonExpressionWithRect(mapObj *map, int iLayerIndex, const char *pszExpression, rectObj rect);

#endif

Expand Down
29 changes: 21 additions & 8 deletions mapogcfiltercommon.c
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,12 @@ char *FLTGetCommonExpression(FilterEncodingNode *psFilterNode, layerObj *lp)
}

int FLTApplyFilterToLayerCommonExpression(mapObj *map, int iLayerIndex, const char *pszExpression)
{
return FLTApplyFilterToLayerCommonExpressionWithRect(map, iLayerIndex, pszExpression, map->extent);
}

/* rect must be in map->projection */
int FLTApplyFilterToLayerCommonExpressionWithRect(mapObj *map, int iLayerIndex, const char *pszExpression, rectObj rect)
{
int retval;
int save_startindex;
Expand All @@ -707,18 +713,25 @@ int FLTApplyFilterToLayerCommonExpression(mapObj *map, int iLayerIndex, const ch
map->query.maxfeatures = save_maxfeatures;
map->query.only_cache_result_count = save_only_cache_result_count;

map->query.type = MS_QUERY_BY_FILTER;
map->query.mode = MS_QUERY_MULTIPLE;

msInitExpression(&map->query.filter);
map->query.filter.string = msStrdup(pszExpression);
map->query.filter.type = MS_EXPRESSION; /* a logical expression */
map->query.layer = iLayerIndex;

/* TODO: if there is a bbox in the node, get it and set the map extent (projected to map->projection */
map->query.rect = map->extent;
map->query.rect = rect;

retval = msQueryByFilter(map);
if( pszExpression )
{
map->query.type = MS_QUERY_BY_FILTER;
msInitExpression(&map->query.filter);
map->query.filter.string = msStrdup(pszExpression);
map->query.filter.type = MS_EXPRESSION; /* a logical expression */

retval = msQueryByFilter(map);
}
else
{
map->query.type = MS_QUERY_BY_RECT;
retval = msQueryByRect(map);
}

return retval;
}
Expand Down
Binary file added msautotest/wxs/data/point_2_49.dbf
Binary file not shown.
Binary file added msautotest/wxs/data/point_2_49.shp
Binary file not shown.
Binary file added msautotest/wxs/data/point_2_49.shx
Binary file not shown.
Binary file added msautotest/wxs/data/point_3_50.dbf
Binary file not shown.
Binary file added msautotest/wxs/data/point_3_50.shp
Binary file not shown.
Binary file added msautotest/wxs/data/point_3_50.shx
Binary file not shown.
Binary file added msautotest/wxs/data/shp_tileindex_of_shp.dbf
Binary file not shown.
Binary file added msautotest/wxs/data/shp_tileindex_of_shp.shp
Binary file not shown.
Binary file added msautotest/wxs/data/shp_tileindex_of_shp.shx
Binary file not shown.
33 changes: 33 additions & 0 deletions msautotest/wxs/expected/wfs_shp_tileindex_of_shp_bbox_filter.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
Content-Type: text/xml; subtype="gml/3.2.1"; charset=UTF-8

<?xml version='1.0' encoding="UTF-8" ?>
<wfs:FeatureCollection
xmlns:ms="http://mapserver.gis.umn.edu/mapserver"
xmlns:gml="http://www.opengis.net/gml/3.2"
xmlns:wfs="http://www.opengis.net/wfs/2.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://mapserver.gis.umn.edu/mapserver http://localhost/path/to/wfs_simple?SERVICE=WFS&amp;VERSION=2.0.0&amp;REQUEST=DescribeFeatureType&amp;TYPENAME=ms:test&amp;OUTPUTFORMAT=application%2Fgml%2Bxml%3B%20version%3D3.2 http://www.opengis.net/wfs/2.0 http://schemas.opengis.net/wfs/2.0/wfs.xsd http://www.opengis.net/gml/3.2 http://schemas.opengis.net/gml/3.2.1/gml.xsd"
timeStamp="" numberMatched="1" numberReturned="1">
<wfs:boundedBy>
<gml:Envelope srsName="urn:ogc:def:crs:EPSG::4326">
<gml:lowerCorner>49.00000 2.00000</gml:lowerCorner>
<gml:upperCorner>49.00000 2.00000</gml:upperCorner>
</gml:Envelope>
</wfs:boundedBy>
<wfs:member>
<ms:test gml:id="test.1">
<gml:boundedBy>
<gml:Envelope srsName="urn:ogc:def:crs:EPSG::4326">
<gml:lowerCorner>49.00000 2.00000</gml:lowerCorner>
<gml:upperCorner>49.00000 2.00000</gml:upperCorner>
</gml:Envelope>
</gml:boundedBy>
<ms:msGeometry>
<gml:Point gml:id="test.1.1" srsName="urn:ogc:def:crs:EPSG::4326">
<gml:pos>49.00000 2.00000</gml:pos>
</gml:Point>
</ms:msGeometry>
</ms:test>
</wfs:member>
</wfs:FeatureCollection>

2 changes: 1 addition & 1 deletion msautotest/wxs/wfs_200_allgeoms.map
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
# RUN_PARMS: wfs_200_allgeoms_getfeaturebyid.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&STOREDQUERY_ID=urn:ogc:def:query:OGC-WFS::GetFeatureById&ID=point.1" > [RESULT_DEVERSION]
# RUN_PARMS: wfs_200_allgeoms_exception_invalidquery.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&STOREDQUERY_ID=invalidquery" > [RESULT_DEVERSION]
# RUN_PARMS: wfs_200_allgeoms_exception_myquery_missing_param.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&STOREDQUERY_ID=myquery" > [RESULT_DEVERSION]
# RUN_PARMS: wfs_200_allgeoms_myquery.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&STOREDQUERY_ID=myquery&LONGMIN=2&LATMIN=49&LONGMAX=2&LATMAX=49" > [RESULT_DEVERSION]
# RUN_PARMS: wfs_200_allgeoms_myquery.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&STOREDQUERY_ID=myquery&LONGMIN=1.99&LATMIN=48.99&LONGMAX=2.01&LATMAX=49.01" > [RESULT_DEVERSION]
# RUN_PARMS: wfs_200_allgeoms_post_several_getfeaturebyid.xml [MAPSERV] [POST]<GetFeature xmlns="http://www.opengis.net/wfs/2.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="2.0.0" service="WFS" xsi:schemaLocation="http://www.opengis.net/wfs/2.0 http://schemas.opengis.net/wfs/2.0/wfs.xsd"><StoredQuery id="urn:ogc:def:query:OGC-WFS::GetFeatureById"><Parameter name="ID">point.1</Parameter></StoredQuery><StoredQuery id="urn:ogc:def:query:OGC-WFS::GetFeatureById"><Parameter name="ID">multipoint.1</Parameter></StoredQuery></GetFeature>[/POST] > [RESULT_DEVERSION]
# RUN_PARMS: wfs_200_allgeoms_post_exception_invalidquery.xml [MAPSERV] [POST]<GetFeature xmlns="http://www.opengis.net/wfs/2.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="2.0.0" service="WFS" xsi:schemaLocation="http://www.opengis.net/wfs/2.0 http://schemas.opengis.net/wfs/2.0/wfs.xsd"><StoredQuery id="invalidquery"></StoredQuery></GetFeature>[/POST] > [RESULT_DEVERSION]

Expand Down
86 changes: 86 additions & 0 deletions msautotest/wxs/wfs_shp_tileindex_of_shp.map
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#
# Test WFS : shapefile tileindex of shapefiles (#5291)
#
# REQUIRES: INPUT=OGR SUPPORTS=WFS
#
# In the logs, point_3_50.shp shouldn't appear
# RUN_PARMS: wfs_shp_tileindex_of_shp_bbox_filter.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&REQUEST=GetFeature&SERVICE=WFS&TYPENAMES=ms:test&VERSION=2.0.0&FILTER=<Filter><BBOX><Envelope srsName='urn:ogc:def:crs:EPSG::4326'><lowerCorner>48.9 1.9</lowerCorner><upperCorner>49.1 2.1</upperCorner></Envelope></BBOX></Filter>" > [RESULT_DEVERSION]

MAP

NAME WFS_TEST
STATUS ON
SIZE 400 300
EXTENT 1 48 4 51
IMAGECOLOR 255 255 255
SHAPEPATH "data"

#
# Start of web interface definition
#
WEB

IMAGEPATH "/tmp/ms_tmp/"
IMAGEURL "/ms_tmp/"

METADATA
"ows_updatesequence" "123"
"wfs_title" "Test simple wfs"
"wfs_onlineresource" "http://localhost/path/to/wfs_simple?"
"wfs_srs" "EPSG:4326"
"ows_abstract" "Test WFS Abstract"
"ows_keywordlist" "ogc,wfs,gml,om"
"ows_service_onlineresource" "http://localhost"
"ows_fees" "none"
"ows_accessconstraints" "none"
"ows_addresstype" "postal"
"ows_address" "123 SomeRoad Road"
"ows_city" "Toronto"
"ows_stateorprovince" "Ontario"
"ows_postcode" "xxx-xxx"
"ows_country" "Canada"
"ows_contactelectronicmailaddress" "tomkralidis@xxxxxxx.xxx"
"ows_contactvoicetelephone" "+xx-xxx-xxx-xxxx"
"ows_contactfacsimiletelephone" "+xx-xxx-xxx-xxxx"
"ows_contactperson" "Tom Kralidis"
"ows_contactorganization" "MapServer"
"ows_contactposition" "self"
"ows_hoursofservice" "0800h - 1600h EST"
"ows_contactinstructions" "during hours of service"
"ows_role" "staff"
"ows_enable_request" "*"
END
END

PROJECTION
"init=epsg:4326"
END


#
# Start of layer definitions
#


LAYER
NAME test
CONNECTIONTYPE OGR
TILEINDEX "shp_tileindex_of_shp.shp"
TILEITEM "LOCATION"
METADATA
"wfs_title" "test"
"wfs_description" "test"
"wfs_featureid" "id"
END
TYPE POINT
STATUS ON
EXTENT 1 48 4 51
PROJECTION
"init=epsg:4326"
END

DUMP TRUE
END # Layer


END # Map File

0 comments on commit 9f1ef95

Please sign in to comment.