Re: [Geotools-devel] about wiping out invalid fids from queries

[Andrea A. <aa...@op...>]

Gabriel Roldán ha scritto:
> Hi Andrea,
> 
> giving a run at simplifying filters for jdbc datastores (for instance, postgis) 
> I have some doubts on where it'd be best to do so, since I'm not an expert on 
> the jdbc datastores I thought I may use your thoughts once more to not mess it 
> up :)
> 
> So, we have this nice issue (http://jira.codehaus.org/browse/GEOT-2182) about 
> wiping out invalid fids that aims to fix what seems to be a pretty common 
> mistake on WFS's, send a fid filter over typeName1 with a fid like 
> "typeName2.someting" and you'll still get a non empty result (I have seen the 
> defect on a commercial one).
> 
> But for jdbc ones I guess we should add a isValid(String fid):boolean method to 
> FIDMapper and then feed the SimplifyingFilterVisitor with an adaptor 
> FIDMapper->FIDValidator.
> This way related things keep close instead of requiring a big switch statement 
> in order to decide which validation strategy is appropriate for a given 
> fidmapper.
> 
> Now, about where to apply this, for postgis I guess it would be at 
> DefaultSQLBuilder.splitFilter(Filter filter), but I couldn't find out where to 
> get the FIDMapper in use at this point. The closer I get to is that SQLBuilder 
> contains a SQLEncoder which in turn has a setFidMapper(FIDMapper) mehod, but 
> no getter. So... I'm open to any suggestion on how we should better hanlde 
> this for postgis and jdbc in general. please?

Well, if you look at the current code you'll see that the simplifying
filter visitor has been used only in JDBCFeatureStore.getFeatureReader.
There you can get the fid mapper, but as you have noticed, it's not
a general enough position, as it leaves out FeatureSource methods
such as bulk updates/deletes, and in general all writes.

Given that SqlEncoder is fid mapper away anyways your proposal to
expose the fid mapper in a getter sounds good, and solves the
above by allowing other places to benefit from the filter
simplification.

The best place to inject the fid mapper seems to be
JDBC1DataStore.getSqlBuilder(), but pay attention,
that method is overridden in most JDBC data stores.

Given the magnitude of this change I would be more comfortable
about:
- having someone review it
- commit it on 2.5.x only after GeoServer 1.7.1 is released

Cheers
Andrea

-- 
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.