Jody Garnett ha scritto:
> Andrea Aime wrote:
>> First thing is, why are there two ways to compare FeatureTypes for 
>> equality?
>> The first one is FeatureType.equals(xxx), the second one is 
>> DataUtilities.compare(ft1, ft2).
>>   
> Andrea, everything in DataUtilities were tools needed in the heat of the 
> moment to get the job done.  It could be that FeatureType.equals( xxx ) 
> was eventually "fixed" and DataUtilities.compare( ft1, ft2 ) is no 
> longer needed.
> 
> It is also likely that DataUtilities method is doing something a little 
> specific needed for GeoServer (all these methods date from early 
> GeoServer work).
>> Some places use the first, others the second, maybe in the same 
>> algorithm (bad idea...).
>>   
> Gah.
> Correct: use FeatureType.equals( xxx ), with the understanding that the 
> FM may change the scope of what a FeatureType is and upset code.
> Safe: use DataUtilities.compare as it is a method we can ensure works 
> with the new feature model (say only checking the data compatibility of 
> the two feature types)

Hmmm... I'm more inclined to go the correct way... guess I'll have
to compare DataUtilities.compare() and DefaultFeatureType.equals() a 
little more and spot where the differences are:
http://jira.codehaus.org/browse/GEOT-1099

>> Second, why DefaultAttributeType.equals(...) does not take into 
>> account the restriction when evaluating equality?
>>   
> Restrictions were added by David Zwieres and Chris Holmes, I suspect 
> they just missed it.

Chris? Feature types with different restrictions should not be equal,
or do you have any reason to say they are equals anyways? In that
case, an external comparator like DataUtilities.compare(ft1, ft2)
would be preferable I guess, provided that we can specify in detail
what's the subject of comparison.

>> Finally, the bug. If I invoke getFeatureReader(FeatureType 
>> requestedType, Filter filter, Transaction t) on a JDBC data store
>> and requestedType is a subset of the datastore's schema, I'm
>> guaranteed to get an exception from the ReTypeFeatureReader,
>> since the method builds a query that already extracts the
>> requested attributes but... there is a but in my opinion.
>>   
> Okay I understand that .. I would hope that ReTypeFeatureReader is not 
> checking things carefully and just calling create( Object[]) using your 
> provided FeatureType :-(

Hmm.... not sure about that. Strict checking allowed us to find
this inconsistency as well as few other bugs.

>> What if the requested type had different restrictions on attributes?
>> Would it be ok to return a feature reader sporting a feature type
>> that's different (restrictions wise) from the one requested?
>>   
> Depends on your view of valid vs invalid data. Should invalid data 
> exist? Should we allow it to be created?
> Currently we do not allow invalid data to be created (let alone 
> handled), the FeatureType represents a *contract* or assurance to 
> consumers of the data (say an XMLTransform) that only valid data will be 
> provided.
> 
> Difficulty is that invalid data does in fact exist, and as such we 
> should be able to model it (even if just to clean it up).

I guess if we anticipate invalid data (import routines, something
similar) we can just read with a more relaxed feature type, clean up,
and then retype to one with stricter checks. This also requires the 
ReTypeFeatureReader understands the difference between restrictions 
thought :-p

> Answer:
> Make ReTypeFeatureReader skip over any content that is "invalid" against 
> those restrictions, and log the warnings. This should already happen as 
> part of that FeatureType.create( Object[] ) step I guess. May need to 
> check to see if the reader gives up at the first failure, or keeps 
> trying ...

I'm uncomfortable with something that just skips and logs. I prefer
something that throws exceptions as a default behaviour.
What you ask for seems a new class to me, that does filtering.
Could be a FilteringReTypeFeatureReader, or we could just have a flag
that makes the ReTypeFeatureReader lenient and skip instead of
getting upset :-)

Cheers
Andrea

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to