Andrea Aime ha scritto:

>> This is a dammed if you do dammed if you don't - the length information 
>> is needed if we are going to export from PostGIS into a shapefile 
>> correctly.
> 
> Well, broken build is in my book more damned, it prevents any
> other work in the module. It's the first single
> thing a developer should never break, and it's usually part
> of the dev responsibility to fix the build he broke as well.
> So if you don't, I'm going to revert the change and open
> a jira about it (let's call it a blocker if you want) to fix
> things on trunk.

So I did a bit of research looking for a quick solution (something
that would not require a day of work)... and this does not look
good. The single line change you made broke the tests in:
- postgis
- versioned postgis
- mysql
And I could not verify DB2.
All tests trying to compare with each other:
- features
- feature types
- attribute types
are failing because the feature types declared in
DataTestCase do not have a specific length, and the
attribute type equality fails there since the ones
coming from the databases do.
FeatureType equals cascade on attribute type
equals, and feature equals cascades on feature type
equals, so you can see how ugly this is becoming.

We could try to change DataTestCase so that the reference
feature types have the proper length, but that's going
to be troublesome because of the way you implemented
things anyways. Look at the code (from
JDBC1DataStore.buildAttributeType())

AttributeTypeBuilder atb = new AttributeTypeBuilder();
atb.setName(columnName);
atb.setBinding(type);
atb.setMinOccurs(min);
atb.setMaxOccurs(1);
atb.setLength( rs.getInt( LENGTH ) );
return atb.buildDescriptor(columnName);

As you can see there is no type check, this means
the thing is getting applied to all data types, not only
to strings (in fact the tests are failing because of
the length constraint being applied to numeric data).

Even restricting length limitations to strings, there
is a need to:
- change TestData so that it has the proper restrictions
   on string data types
- verify with all of the above that the tests are working
   fine again
- verify that changes to DataTestCase do not break other tests
   using them (there are quite a few).

Another way out is stating that restrictions checks should
not be part of the PropertyTypeImpl equality code (which
is where the equality test do fail now).

In any case that's a sizeable amount of work, not something
I can do in a spare hour, so for the moment I'm reverting
that change.

Once you decide what length restrictions you do really need
(only strings, also numeric, what about dates?) can you
open a jira issue to fix back this length issue? (which
is totally new btw, I checked in 2.4.x with the old feature
model, there was no length restriction being set in the code,
so what you added is a new feature).

Cheers
Andrea

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

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to