MapScript $layer->whichShapes() does not take the layer filter/filteritem into account

[sdlime]

The method whichShapes() doesn't work if a layer FILTER or FILTERITEM is set. The reason is that the function msLayerWhichItems() has typically not been run so the code doesn't have access to the right attributes on which to evaluate an expression.

The problem is the code in layer.i (and presumably the PHP version too). The method should look something like (need error checking around msLayerWhichItems()):

int whichShapes(rectObj rect) {

msLayerWhichItems(self, MS_TRUE, MS_FALSE, NULL); return msLayerWhichShapes(self, rect);

}

the msLayerGetItems() call that is currently there is unnecessary since it is called when a layer is opened. A typical sequence would be:

$layer->open();
$layer->whichShapes($map->{extent});

while ($shape = $layer->nextShape()) {
  # do something with the shape
}
$layer->close();

[sdlime]

Fixed in Swig/MapScript (r7871). Still needs fixing in PHP/MapScript so reassigning...

Steve

[dmorissette]

Fixed. I have ported the change to PHP MapScript in SVN trunk r7879.

(Found this one by accident... it wasn't CC'd or assigned to me)

[sdlime]

I've noticed that changing the component doesn't change the assignee. Is that how it's supposed to work?

[sdlime]

Reopening. The fix gets the right shapes but not all the items. Need a way to ensure ALL items are available (what msLayerGetItems() does) and that the filter processing variables are populated (what msLayerWhichItems() does).

Steve

[tamas]

Any update on this one?

It looks like these changes broke my existing code, and I need to call layer.open() right after the whichShapes call in order to retrieve any items from the data source, which is annoying.

I understand however that filteritemindex (and the others) should also be set somehow. I would probably suggest to set 'layer->connectiontype = MS_INLINE' temporarily before calling whichItems so as to force adding all items to the list.

Tamas

[sdlime]

No, haven't dug in. I'd like to propose a 5.4.1 soon so I better get to it...

Steve

[sdlime]

I've thought about this a good bit. It's basically a manifestation of one-pass queries issues. This method wasn't intended to replace the query methods but was a simply a quick way to get at a list of shapes in a bbox. We can't deliver shapes with all attributes present and FILTER processing, not in 5.4. I've already written a new version of msLayerWhichItems() in the one-pass sandbox that would work around that I belive.

Best we can do is:

1) give the user all attributes associated with FILTER/EXPRESSION/LABEL operations and change:

msLayerWhichItems(self, MS_TRUE, MS_FALSE, NULL) to msLayerWhichItems(self, MS_TRUE, MS_TRUE, NULL)

If someone were sneaky they could write a TEXT expression like:

TEXT ([item1] ... [itemn])

and force getting 'em all.

or 2) revert to the pre-5.4 version and bail on FILTERs. However, that seems more wrong than option 1.

I don't believe we can simply call msLayerGetItems() after the msLayerWhichItems() because I'm not sure the index values will point to the right item. Shapefiles will since we read the whole record but I've no idea about the other drivers.

Tamas, I'm surprised you can't access any items unless you're not doing any filtering or classification. Probably the case with inline shapes. What do you think?

Steve

[tamas]

Replying to sdlime:

I don't believe we can simply call msLayerGetItems() after the msLayerWhichItems() because I'm not sure the index values will point to the right item. Shapefiles will since we read the whole record but I've no idea about the other drivers.

Steve,

I wanted to mention adding something like in layer.i could probably fix this issue:

int whichShapes(rectObj rect)

{

int oldconnectiontype = self->connectiontype;

self->connectiontype = MS_INLINE;

if(msLayerWhichItems(self, MS_TRUE, MS_FALSE, NULL) != MS_SUCCESS) {

self->connectiontype = oldconnectiontype;

return MS_FAILURE;

}

self->connectiontype = oldconnectiontype;

return msLayerWhichShapes(self, rect);

}

I didn't test this, but hopefully it should work with the current version.

Tamas, I'm surprised you can't access any items unless you're not doing any filtering or classification. Probably the case with inline shapes. What do you think?

I'm afraid I couldn't catch this. By calling msLayerWhichItems the items are cleared and layer.numitems is set to 0 by default, and you cannot access any items until you refer some of those explicitly in the layer definition or call layer.open right after the msLayerWhichItems call. Formerly msLayerGetItems() retieved all items (except for the OGR special fields).

Tamas

[sdlime]

I'm at a loss why that would work. Not doubting you, just don't know at the surface why this would work. You're still calling msLayerWhichItems() which towards the top of the function clears the items regardless of connection type.

msLayerWhichItems() does clear the item list in order to build a new one sufficient to label or classify features. If you're not doing that then you wouldn't need attributes.

Steve

[tamas]

Replying to sdlime:

I'm at a loss why that would work. Not doubting you, just don't know at the surface why this would work. You're still calling msLayerWhichItems() which towards the top of the function clears the items regardless of connection type.

msLayerWhichItems() does clear the item list in order to build a new one sufficient to label or classify features. If you're not doing that then you wouldn't need attributes.

Steve,

In msLayerWhichItems the corresponding code looks something like this (maplayer.c Line 495):

if (layer->connectiontype == MS_INLINE) {

msLayerGetItems(layer);

if (nt > 0)

layer->items = (char )realloc(layer->items, sizeof(char *)*(layer->numitems + nt));

} else {

rv = layer->vtable->LayerCreateItems(layer, nt);

if (rv != MS_SUCCESS) {

return rv;

}

} In order to support the classification and labeling, we retrieve all items by default for the inline layers. If you look forward in the code (line 510-565) you see that the itemindexes are also handled and set to the desired values in this special case. I've already tested this code for various inline layers and seemed to be working ok. I thought our objective is almost the same in the mapscript whichShapes API, which was the original behaviour of this function.

Tamas

[sdlime]

Tamas: Good call, I missed that code when scanning msLayerWhichShapes(). I've tested on shapefiles via MapScript (Perl) and it works fine, with or without filters being set. I've committed the change in the 5.4 branch in r9060.

Dan, can you apply to PHP MapScript? I'll release 5.4.1 as soon as that happens.

Steve

[dmorissette]

Patch applied to PHP MapScript in branch-5-4 r9062 and trunk r9063.

This still needs to be applied to swiginc/layer.i in trunk

[sdlime]

This will change once more in 6.0 if the new version of msLayerWhichItems() in the single-pass sandbox is used. I fixed in trunk r9064... Closing.

Steve