On Tue, Jul 17, 2012 at 3:58 PM, Gabriel Roldan <grol...@opengeo.org> wrote:

> Patch attached to <http://jira.codehaus.org/browse/GEOT-4205>
>
> Please review and comment at your earliest convenience.
>

Seems good, but the implementation of the InternalVolatileFunction seems
broken to me here:

/**
     * @see
org.opengis.filter.expression.InternalFunction#duplicate(org.opengis.filter.expression.Expression[])
     */
    @Override
    public InternalFunction duplicate(Expression... parameters) {
        return this;
    }

Say that foo is an internal volatile function, and you have foo(5 + 7),
the simplifier can legitimately turn 5 +7 into 12 and call duplicate
with just Literal(12) as the parameter, and you are going to ignore that.

Other duplicators we have around do reprojection of geometries in spatial
filters and the like, and they alter the geometry literals and wrap
functions
with on the fly reprojectors, if one of this modification happens to hit
a parameter of your volatile internal function the above will simply ignore
it,
which introduces a bug.

The duplicate method should be implemented function by function returning
a copy of the function with the appropriate parameters.

If the catalog related functions are parameter-less and you know that you
should probably create a base class that's specific to those, and one that
throws an illegal argument exception if the parameter list passed to
duplicate
is not empty

Cheers
Andrea

-- 
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
information.
==

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:   +39 0584 962313
mob:   +39  339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to