RFC 33: Removing msLayerWhichItems()...

[Steve Lime]

Thanks for the feedback. Comments inline...

>>> On 7/10/2007 at 7:43 AM, in message
<f3b73b7d0707100543r69a3270ck80d1ca24226afdfc at mail.gmail.com>, Tamas Szekeres
<szekerest at GMAIL.COM> wrote:
> Steve,
> 
> I'm a bit scared about the complexity of the changes, and I feel not
> all of the relations have been covered by this RFC.

Scares me a bit too. This was just a first crack at describing the change. I'm worried most
about a driver like PostGIS since it allows explicitly setting returned items in sub-selects. I
don't know, for instance, if it incorporates values like LABELITEM or CLASSITEM into the
SQL it eventually submits to PostgreSQL. 

> 1. It seems we would let the drivers set the items array implicitly
> upon the LayerOpen call. It would require to change the current
> behaviour of all of the existing drivers.

Yes, but this may be a small change since the function msLayerGetItems
already exists for each layer. 

> 2. All of the msLayerWhichItems related code (in mapdraw.c,
> maplayer.c, mapogcfilter.c mapquery.c, mapraster.c, mapshape.c,
> mapswf.c) should be altered.

Yup, those calls would go away.

> 3. Not sure about whether the join functionality (in mapjoin.c) involved or 
> not.

Unaffected...

> 4. itemindex related code should be altered in the following files
> (maplayer.c, mapogr.cpp, mapraster.c, maprasterquery.c, mapshape.c)
>  It seems that the tileitemindex has a special role and might be set
> by the driver. Some of the drivers are also involved in setting
> filteritemindex

I was aware of these, just forgot to mention them (it was late).

> 5. styleObj.rangeitemindex is currently exposed to mapscript (should
> probably be hidden)

Agreed. The rangeitemindex would be populated in mapdraw.c in msDrawShape()...

> 6. Not sure about the inline layers, it does not implement GetItems
> currently so msLayerWhichItems might not be eliminated entirely. 

Inline layers don't have items. This change might actually allow for them. We could
allow inline layers to define the items array in the mapfile and via mapscript.

> Wouldn't it be more feasible to keep msLayerWhichItems which would be
> responsible to call msLayerGetItems to set up the items array by the
> drivers, and set the itemindex values as it stands now. In this regard only one
> function would be altered.

There are other benefits for setting item index values during processing, primarily
surrounding changing attribute bindings or expressions. It sucks to have to make 
sure you call a method (e.g. prepareMap) that populates item indexes before
doing something.

> I'm not sure about the final goal (removing double-pass query) either.
> According to the previous posts we would keep the shapeObj-s inside
> the resultcacheMemberObj-s but we would not want to keep the
> compatibility with the existing scripts (which use GetShape call to
> retrieve the members).
> Instead we would want to introduce a new function to retrieve the
> shapes from the resultcache.

I don't know that you'd need a new function. The shapeObj would be 
there alongside the shapeindex and tileindex values. That's another 
RFC though... ;-)

> In addition we address caching only the results of a query and it will
> eventually involve 2 passes (one for the query and one for the further
> rendering by drawQuery for example)

I don't know that you can get away from looping through result sets
multiple times unless queries were tightly integrated into msLayerWhichShapes
and msLayerNextShape. I have thought about doing just that. That is,
you move the filtering found in the msQueryBy...() functions out to the
drivers. Problem is that complicates the drivers and potentially causes
performance problems with simple drawing. Plus there are features like
class filtering and templating that don't work in that scenario. So I think
it's best to leave the query functions as is. There are always opportunities
to move more processing off to the more capable backends within the 
query functions.

> I generally support the intents of this RFC and indeed it would
> simplify how the shapes are handled inside the mapserver core,
> regardless of that RFC22a itself doesn't rely on these changes.

I think RFC22a would certainly benefit from a simplified notion of
a shape in the core though...

I think this would be a good candidate for a branch to test the
implementation...

> Best regards,
> 
> Tamas
> 
> 2007/7/10, Steve Lime <Steve.Lime at dnr.state.mn.us>:
>> Hi all: I tried to put some thoughts down regarding removing 
> msLayerWhichItems() from maplayer.c. This sets us up for feature caching, 
> both using Tamas' nested layers and for single pass query support using the 
> resultCacheMemberObj.
>>
>> Please see: http://mapserver.gis.umn.edu/development/rfc/ms-rfc-33 
>>
>> Steve
>>