Thanks for taking the discussion to the email list Justin; I did warn the patch
author that discussion needed to be public (and would have to wait until after
foss4g).
The one thing I like about the suggestion is not hard coding the jxpath case as
a fall back position.
The concerns I had were:
- performance implications (not sure if that has been measured)
- doing too much work in the canHandle method (this may be the same performance
consideration). I like the idea of canHandle looking at the expression and
short listing ... I don't want it to try and do all the work each time
- use of exception for flow control. Returning null is not really unambiguous
either; perhaps introducing another method would work?
Am I correct in thinking the current implementation is broken with respect to
the bug report? Or is the bug report itself wrong? Should we expect an error to
be returned when an invalid column name is provided? Or should we just return
null to indicate that there was nothing available for the provided xpath
expression?
Currently we have:
canHandle( String xpath ) - used to shortlist
execute( String xpath ) - access the content; may return null if content is not
available
Adding an extra method would look like:
canHandle( String xpath ) - used to short list
canExecute( String xpath ) - used to test if the content is there
execute( String xpath ) - used to access the content; may return null if the
content is null (or is not available)
For reference the definition of PropertyIsNull is
The <PropertyIsNull> element encodes an operator that checks to see if the
value of its content is NULL. A NULL is equivalent to no value present. The
value 0 is a valid value and is not considered NULL.
Section 6.3 has a whole bunch of stuff on the xpath expressions suitable for
use with PropertyName; however they are all positive examples showing how to
query existing content; what we need is an example showing how to fail. I think
the assumption of returning "null" when the content is not available at the
requested xpath expression is a good one (it does represent that no information
is present after all; and the PropertyIsNull filter is available to make use of
this information).
Jody
On 21/09/2010, at 11:18 AM, Justin Deoliveira wrote:
> Hi all,
>
> Recently this patch has been proposed:
>
> http://jira.codehaus.org/browse/GEOT-3066
>
> And this issue was discussed in detail on the mailing list some time ago in
> this thread:
>
> http://osgeo-org.1803224.n2.nabble.com/evaluating-invalid-attribute-names-td5489979.html
>
>
> Apologies for not weighing in then. But I think this has some pretty serious
> backward compatibility issues. The issue/proposal is to throw an exception vs
> return null when a type has no such attribute described by an attribute name
> or xpath, rather than return null. The thread above proposes to just change
> the behaviour and update any test cases that rely on it. I could not disagree
> more with his approach. This breaks backward compatibility and i think
> requires more discussion. There is much more use of geotools beyond test
> cases and even beyond geoserver and udig. If there is one thing i learned at
> foss4g it is that there are lots of people using geotools, even though they
> don't take parts in discussions on the developer list.
>
> So i think this change requires more discussion and probably a proposal. And
> it certainly can't take place on stable branch imo.
>
> Is there any way we can have this behaviour engage only when the user
> explicitly requests it? That has been a pattern that has worked very well in
> the past for such changes.
>
> -Justin
>
> --
> Justin Deoliveira
> OpenGeo - http://opengeo.org
> Enterprise support for open source geospatial.
>
> ------------------------------------------------------------------------------
> Start uncovering the many advantages of virtual appliances
> and start using them to simplify application deployment and
> accelerate your shift to cloud computing.
> http://p.sf.net/sfu/novell-sfdev2dev_______________________________________________
> Geotools-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
