Carsten: I think you proposal is clear, and the way to move forward to send it to the email list and call for a vote. We have the procedure in place *just so you are not stuck waiting for feedback* ... and I am afraid you have been stuck waiting for feedback. The idea is that anyone like me who has hesitations needs to get out of your way, or send you a -1 feedback with an alternate proposal for you to be successful.
Reading https://github.com/geotools/geotools/wiki/SQL-Encoding-of-Vendor%E2%80%90Provided-Custom-Functions - You explain the existing internal-core-functions well; and how the proposal does not affect that - You explain the custom vendor-provided functions clearly; and the desire to do so with a tag or annotation to make this less bother - The proposal is not complete - Tasks are not filled in; so I am not what is required, and if you need help or collaborators to be successful - API Change is not filled in so I do not know exactly what you are proposing I understand the desire, and all of my feedback amounts to ensuring that we account for the wider systems - Have to be careful not get get dependencies from the function to the datastore - We cannot have a dependency from the function implementation, to an interface in a specific datastore - We should not assume the datastore is available on the classpath , the function may be used on its own - The FilterCapabilities are processed by GeoServer to come up with a function list; need to ensure we do not mess that API contract up. A lot of that contract will be in javadocs; perhaps not expressed clearly (as they often assume familiarity with the WFS specification) - Andrea pointed out a few places where we already know there is a problem (some PostGIS specific functions) <-- this is a good test of your proposed change? Can it handle these two functions and fix some technical debt that occurred when we were not being careful about function boundaries... I really need to see the API Change BEFORE / AFTER section (that way it is not so hypothetical). Let me try and see if I can understand your proposal, by using what you describe to "fix": https://github.com/geotools/geotools/blob/main/modules/plugin/jdbc/jdbc-postgis/src/main/java/org/geotools/data/postgis/filter/FilterFunction_pgNearest.java I am going to use an annotation rather than an tag interface, since I do not want introduce a dependency from the generic function to a specific DataStore. While I know some function may defined alongside a DataStore, I always want the freedom to move these into core, incase another database has similar functionality. If we force each database to make its own function then are ensuring we get duplication by design. *API CHANGE* *BEFORE:* createFilterCapabilities(boolean encodeFunctions){ ... // n nearest function caps.addType(FilterFunction_pgNearest.class); public class FilterFunction_pgNearest extends FunctionExpressionImpl implements VolatileFunction { public static FunctionName NAME = new FunctionNameImpl( "pgNearest", Boolean.class, // required parameters: FunctionNameImpl.parameter("geometry", Geometry.class), FunctionNameImpl.parameter("num_features", Integer.class)); public FilterFunction_pgNearest() { super(NAME); } @Override public Object evaluate(Object feature) { throw new UnsupportedOperationException("Unsupported usage of Nearest operator"); } } *AFTER:* The "FilterFunction_" was done by an early code generator and was not intended to be copied everywhere: class FiltertoSqlHelper { createFilterCapabilities(boolean encodeFunctions) ... // n nearest function // we need to process all functions looking for annotations matching "postgis' ... } public Object visit(Function function, SQLMapping encode, NearestHelperContext ctx) { // check the mapping for dbtype and function mapping if (encode.getMapp() == "knn) { // map to <-> operator } } /** * List of the nearest features to the provided geometry. * For more information see https://postgis.net/docs/geometry_distance_knn.html */ @SQLMapping{"postgis","knn") public class KNearestNeighbourFunction extends FunctionExpressionImpl implements VolatileFunction { // too bad we do not have a way to deprecate function names public static FunctionName NAME = new FunctionNameImpl( "pgNearest", Boolean.class, // required parameters: FunctionNameImpl.parameter("geometry", Geometry.class), FunctionNameImpl.parameter("num_features", Integer.class)); public KNearestNeighbourFunction() { super(NAME); } @Override public Object evaluate(Object feature) { throw new UnsupportedOperationException("Unsupported usage of Nearest operator"); } } -- Jody Garnett On Jan 16, 2024 at 7:09:57 AM, Jody Garnett <jody.garn...@gmail.com> wrote: > Thanks for the reminder, you could also reply to the post with a topic for > the agenda :) > > -- > Jody Garnett > > > On Mon, Jan 15, 2024 at 11:04 PM Carsten Klein <c.kl...@datagis.com> > wrote: > >> Hello Jody, >> >> are you going to attend the GeoServer/GeoTools PMC Meeting at 18:30 CET >> / 9:30 PST on January 16th (today)? >> >> If so, maybe you could bring my latest proposal at >> >> >> https://github.com/geotools/geotools/wiki/SQL-Encoding-of-Vendor%E2%80%90Provided-Custom-Functions >> >> to the attention of the GeoTools devs? >> >> I've sent an email to the geotools-devel list on January 9th, 2024 and >> did dot yet receive any response. Do you think there is something wrong >> with the proposal or is it not clear enough? >> >> Actually, the requested/proposed new capabilities are quite important >> for one of our upcoming projects and it would be great to get at least >> some feedback about what the core developers think about it. >> >> To be honest, additionally, i'd love to leave the implementation up to >> the GeoTools core developers, since those are already familiar with the >> contributing and changing procedures of GeoTools. As shown by the >> proposal, it's only about introducing one or two new interfaces plus >> adding a line to each of the database backed data stores. Likely, the >> pure implementation takes about 20 minutes or less. >> >> Cheers, >> >> Carsten Klein >> >> -- >> >> Carsten Klein >> Lead Software Engineer >> >> DataGis GmbH >> >> Johann-Strauß-Str. 26 >> 70794 Filderstadt >> GERMANY >> >> Phone: +49 7158 9490 106 >> Fax: +49 7158 9490 111 >> >> E-Mail: c.kl...@datagis.com >> Internet: www.datagis.com >> >> Commercial Register: Stuttgart, HRB 225945 >> Management: Dr. Gunter Hahn, Markus Ruess, Carsten Klein >> >>
_______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
