Opened 12 years ago

Closed 12 years ago

#3905 closed defect (fixed)

OracleSpatial poor performance due to scollable cursor

Reported by: msmitherdc Owned by: sdlime
Priority: high Milestone: 6.0.1 release
Component: Input - Native Oracle Spatial Support Version: svn-trunk (development)
Severity: major Keywords: rfc 52, query, oracle, performance, optimization
Cc: dmorissette, tamas, aboudreault


As part of the one pass query optimization, the main cursor retrieving data from Oracle Spatial was changed from OCI_DEFAULT to OCI_STMT_SCROLLABLE_READONLY. This adds significant overhead and slows down data retrieval.

It needs to be determined if the one-pass query optimizations actually need the scrollable cursor and if so, determine the best method of only invoking the scrollable cursor under those circumstances.

Location of the OCI directive is

Attachments (1)

3905.062711.patch (17.2 KB ) - added by sdlime 12 years ago.
Patch to add "isQuery" flag to msLayerWhichShapes().

Download all attachments as: .zip

Change History (28)

comment:1 by tamas, 12 years ago

Cc: tamas added

comment:2 by dmorissette, 12 years ago

Cc: dmorissette aboudreault added; dmorisette removed

comment:3 by sdlime, 12 years ago

Any thoughts on this one? I'm not in a position to comment although I could make the change...


comment:4 by msmitherdc, 12 years ago

If we know the map mode (query, nquery, wfs, wms, map) that is being requested, we could invoke the scollable cursor only in the query related situations (as it is needed in those cases) and use the default cursor for mode=map/wms requests.

comment:5 by aboudreault, 12 years ago

I'm going to take a look tomorrow.

comment:6 by sdlime, 12 years ago

I think Tamas had the same issue with the SQL Server driver. Wonder if/how he solved it?


comment:7 by msmitherdc, 12 years ago

This is what Tamas did

This issue is possibly related to the single pass query implementation in the Oracle driver. I've run into a similar issue with mssql2008 when trying to implement the same. In my opinion there's no need to make the cursor scrollable in most cases (like in the normal drawing) since it affects the overall performance in a harmful way. With regards to mssql2008 I've ended up with a second fetch for the query results and use the forward only cursor to get the shapes sequentially then. This results in 2 fetch instead of one but avoid scrollable cursors at all.

comment:8 by aboudreault, 12 years ago

In the function msOracleSpatialLayerWhichShapes, I can check what the map query mode is. So, if the query mode is MS_QUERY_IS_NULL, I use OCI_DEFAULT as cursor. However, I have a few questions:

  • Will this cover all cases? I see that msQueryByFeature doesn't set any query mode. It probably require the scrollable cursor?..
  • I think WFS requests use the internal msQueryByRect and other functions... So they should be fine?
  • An issue with my solution is that if (ie. in mapscript) I execute a whichShapes, the default cursor will be used. but if I do a queryByRect(), then a whichShapes later... the cursor will still be scrollable because the query is still set. Is this an issue at this point?

Could anyone tell me what modes need a scrollable cursor (other than query, nquery) ? I will also have to add some other check. ie. all wms requests don't need a scrollable cursor.

comment:9 by dmorissette, 12 years ago

When do we need the scrollable cursor anyway? I could be wrong, but do the CGI query modes, as well as WFS, etc. not simply do a single sequential scan of the results which is the whole idea of "single-pass query"? Would the scrollable cursor be useful only in the mapscript case then? If that's the case then perhaps MapScript could have an additional arg/hint to request the scrollable cursor?

comment:10 by msmitherdc, 12 years ago

Wfs didn't work for me with the default cursor

comment:11 by sdlime, 12 years ago

I'm assuming the scrolling cursor is needed with query processing. One-pass is a not really true. It's really two-pass instead the old method that did loads of selects by a unique id. Things like GML require a first pass to compile a bounding box and then you have the presentation of actual geometries. The cursor is reset then I guess.

All the query modes need it then I presume. I guess we need a reliable way to know if we're processing a query. The map->query object would be the place to manage it by setting and checking map->query.type. If it's set to MS_QUERY_IS_NULL then you shouldn't need the scrollable cursor.

msQueryByFeatures() actually should already have a query type set before it's called. It assumes another query has been done beforehand (e.g. find all counties with names starting with 'C') and uses those results to generate a new result set. In fact, we could check to make sure the query type is not MS_QUERY_IS_NULL at the top of msQueryByFeatures()...

Queries themselves are easy because msLayerWhichShapes() is called once you know a layer is to be queried so you just look at the map->query.type. It's query maps that present the problem since you might have a query set and then when drawing the querymap (with certain types) you could process layers not impacted by the query and slow things down with the scrollable cursor.

Might consider passing an extra context variable (e.g. MS_CONTEXT_RENDER, MS_CONTEXT_QUERY) to msLayerWhichShapes() and the underlying driver can make use of it if necessary. The function isn't called that many places and it's clear which context to use...


comment:12 by tamas, 12 years ago

Currently the map->query.type is not handled in a consistent way. For example when using drawQuery in highlight mode the background image is drawn also in query mode. I'm also uncertain how these multiple drawing sessions (with switching the cursor type) would work in a connection pooling environment with the OCI driver.

Another question is that do we really make use of the scrollable cursor in a query operation? In most cases we just want to rewind the cursor back to the first record and continue to fetch the records sequentially then. In this case using a second query (to reset the forward only cursor) may probably be more efficient.

comment:13 by sdlime, 12 years ago

map->query.type is probably not cleared consistent enough, should be reset in msLayerClose() if not already.

msLayerWhichShapes() is used to generate a list of candidate shapes. The query functions then refine that list (via msLayerNextShape()) and store row references. It's msLayerGetShape() then that uses the still-open result set (cursor). It's not a rewind. I don't know what makes sense in Oracle's case in this sequence of operations.

The more I think about it the more adding flag to msLayerWhichShapes() makes sense. We could try that in trunk and see how it works?


in reply to:  13 comment:14 by tamas, 12 years ago

Replying to sdlime:

msLayerWhichShapes() is used to generate a list of candidate shapes. The query functions then refine that list (via msLayerNextShape()) and store row references. It's msLayerGetShape() then that uses the still-open result set (cursor). It's not a rewind. I don't know what makes sense in Oracle's case in this sequence of operations.

In most cases the shapes in the resultcache is accessed in the same order at it was retrieved during the query. In this regard there's no need to establish a random access to the resultset, a forward only cursor would also be sufficient (see the OGR and the MSSQL drivers for reference)

By utilizing this sequence the driver just require to store the recent query (statement) and reissue that to the data source in case if the specified resultindex is less then the last resultindex retrieved by the driver. (kinda like a rewind operation)

comment:15 by sdlime, 12 years ago

So what needs to happen here then? These are changes I can't make...


comment:16 by msmitherdc, 12 years ago

The forward only cursor is not sufficient in the WFS case, I get

msOracleSpatialLayerGetShape(): OracleSpatial error. Shape type is null... this probably means a record number was requested that could not have been in a result set (as returned by NextShape)

after changing to forward only cursor.

Steve, If you want to create a patch that adds the flag to msLayerWhichShapes, I can test it out.

comment:17 by sdlime, 12 years ago

I've attached a patch against trunk. I've no way to test Oracle so you may have to tweak maporaclespatial.c just a touch. This basically just adds an "isQuery" flag to msLayerWhichShapes(). The Oracle driver is the only one to check it.


by sdlime, 12 years ago

Attachment: 3905.062711.patch added

Patch to add "isQuery" flag to msLayerWhichShapes().

comment:18 by sdlime, 12 years ago

Mike: Any chance to test?


comment:19 by msmitherdc, 12 years ago

Yes, i'll hopefully get to it this afternoon (which I see it is now, sigh). Maybe this evening

comment:20 by msmitherdc, 12 years ago

All, I've tested this against Oracle using both the default cursor (mode=map) and the forward scrolling cursor (wfs). It worked fine in both cases.

comment:21 by sdlime, 12 years ago

Owner: changed from jimk, sdlime to sdlime
Status: newassigned

Any reason not to commit this fix in the 6.0 branch and trunk. It's a pretty benign change but does alter the C API. I do need to update MapScript, change will be transparent to users... Steve

comment:22 by msmitherdc, 12 years ago

I'm happy with it on my end. I'd love to see some testing by others but it may just be easier to commit and deal with the issues that arrise.

comment:23 by dmorissette, 12 years ago

No objection to backporting to 6.0.1 from my end, but that would have to be done ASAP.

comment:24 by sdlime, 12 years ago

Resolution: fixed
Status: assignedclosed

Committed in r11880 (trunk) and r11881 (branch-6-0). Marking as fixed. Let me know if anyone see's issues...


comment:25 by rouault, 12 years ago

Resolution: fixed
Status: closedreopened

Steve, it seems that r11880 and r11881 broke mssql2008 plugin building. See

At first sight, it seems to be just a typo :

int msMSSQL2008LayerWhichShapes(layerObj *layer, rectObj rect, in isQuery)

instead of

int msMSSQL2008LayerWhichShapes(layerObj *layer, rectObj rect, int isQuery)

comment:26 by sdlime, 12 years ago

Cripes, yes a typo. Will fix... Steve

comment:27 by sdlime, 12 years ago

Resolution: fixed
Status: reopenedclosed

Fixed in r11883 and r11884. -Steve

Note: See TracTickets for help on using tickets.