I just created HHH-10901 for that. So we'd still have the question of merging hibernate-spatial into hibernate-core. Personally I am against that for the dependencies.
On Tue, Jun 28, 2016, 11:02 AM Karel Maesen <ka...@geovise.com> wrote: > Yes, a SqlFunctionContributor would solve the problem of registering > spatial functions. Then the only thing left are the methods defined in the > SpatialDialect interface. These are helper methods for use in Criterion > implementations (to generate the correct SQL syntax). I think these methods > can be refactored to a separate Service (SpatialQuerySupportService). In > that case, yes we can remove the spatial dialects altogether. > > When I'm back from holiday, I will attempt refactoring the methods in > SpatialDialect to a Service. If you then provide a SqlFunctionContributor > we can start with the work of removing spatial dialects. > > > On Tue, Jun 28, 2016 at 5:19 PM, Steve Ebersole <st...@hibernate.org> > wrote: > >> Thanks for the clarification. >> >> However, all told I am not sure what you are suggesting. Are you >> suggesting that we somehow make Dialect itself expandable/mutable and >> hibernate-spatial hook into that to expand mutate the ORM Dialect? >> >> If it is just augmenting Types and SQLFunctions we already have >> capabilities for this that I'd rather leverage: >> >> - For Types, hibernate-spatial would just provide a TypeContributor >> and based on the ORM Dialect it could contribute the needed Types. >> - We do not currently have a clean way to "contribute SQLFunctions" >> from an integration. But if we are making changes anyway, I'd rather do >> this - adding a SqlFunctionContributor >> >> If spatial dialects are really *only* used to contribute Types and >> SQLFunctions, >> the above options would remove the need for having spatial dialects at all >> and still allow hibernate-spatial to contribute its things. >> >> WDYT? >> >> >> On Tue, Jun 28, 2016 at 9:18 AM Karel Maesen <ka...@geovise.com> wrote: >> >>> @Steve, besides Types a number of spatial Functions are registered for >>> use in queries. >>> >>> The Dialect#contributetypes method is currently already used by >>> Hibernate-spatial to inject to correct Types (with the correct >>> SqlTypeDescriptors) for the SpatialDialect. >>> >>> On Tue, Jun 28, 2016 at 3:40 PM, Steve Ebersole <st...@hibernate.org> >>> wrote: >>> >>>> I agree. I think keeping these separate makes the most sense. >>>> >>>> @Karel, we already have that capability wrt Types (see >>>> Dialect#contributeTypes). What else would get injected? >>>> >>>> On Tue, Jun 28, 2016 at 8:33 AM Gunnar Morling <gun...@hibernate.org> >>>> wrote: >>>> >>>>> One thing to keep in mind is that the module system of Java 9 >>>>> ("Jigsaw") doesn't support a notion of optional module dependences. >>>>> >>>>> So if this is meant to remain an "optional feature" (i.e. Geo-specific >>>>> libraries are not required at runtime when not using the spatial stuff), >>>>> it >>>>> should remain a separate artifact - this is, should we plan to make >>>>> Hibernate ORM JARs Jigsaw modules at some point. >>>>> >>>>> --Gunnar >>>>> >>>>> >>>>> 2016-06-28 15:22 GMT+02:00 Karel Maesen <ka...@geovise.com>: >>>>> >>>>>> It makes sense for some Dialects such as those for MySQL and MS SQL >>>>>> Server. >>>>>> Less so for Postgresql and Oracle Spatial because here the spatial >>>>>> capabilities need to be installed and configured separately (and even >>>>>> have >>>>>> versioning separate from the main database engine). >>>>>> >>>>>> I would favour an approach, close to what Sanne is suggesting, where >>>>>> the >>>>>> spatial capability (Incl. Types) is injected into the Dialect during >>>>>> Dialect construction based on some explicit configuration. In that >>>>>> way the >>>>>> relation between Dialect and the spatial capability mirrors the >>>>>> relation >>>>>> between database engine and spatial capability (if any). >>>>>> >>>>>> Regards, >>>>>> >>>>>> Karel >>>>>> >>>>>> PS: I'm leaving tomorrow on holiday for a couple of weeks so I won't >>>>>> be >>>>>> able to resume the exchange before July 20. >>>>>> >>>>>> On Mon, Jun 27, 2016 at 3:53 PM, Steve Ebersole <st...@hibernate.org> >>>>>> wrote: >>>>>> >>>>>> > They have not been "merged" in the same way we merged >>>>>> > hibernate-entitymanager into hibernate-core. So at the moment >>>>>> using >>>>>> > hibernate-core e.g. does not require geolatte to be present. >>>>>> geolatte is >>>>>> > only required if using hibernate-spatial (isolated transitive dep). >>>>>> > >>>>>> > Karel, what are your thoughts on this? I am not a fan of making >>>>>> geolatte >>>>>> > optional/provided, but if we all deem that folding >>>>>> hibernate-spatial into >>>>>> > hibernate-core (ala hibernate-entitymanager) as Vlad suggests is >>>>>> desirable >>>>>> > then I will accept that >>>>>> > >>>>>> > On Mon, Jun 27, 2016, 6:50 AM Sanne Grinovero <sa...@hibernate.org> >>>>>> wrote: >>>>>> > >>>>>> > > Nice idea! >>>>>> > > >>>>>> > > since the modules were merged already, don't we already require >>>>>> > > geolatte-geom ? >>>>>> > > I guess some code might be intentionally designed to fail >>>>>> gracefully >>>>>> > > about this library being there or not, but we'd need to make sure >>>>>> that >>>>>> > > can be tested for it to be maintainable. >>>>>> > > >>>>>> > > My preference would be to have: >>>>>> > > - All Dialects automatically provide the spatial extensions if >>>>>> the >>>>>> > > needed dependencies are in place: we could automatically alias >>>>>> them >>>>>> > > based on this? >>>>>> > > - a good error message naming the missing dependencies explicitly >>>>>> > > when someone attempts to use such a Spatial extensions, but the >>>>>> > > feature was not enabled by our automatic logic. >>>>>> > > - be able to test for these. >>>>>> > > >>>>>> > > In practice I believe this means we should still have it as an >>>>>> > > independent source module, compile and test it as an independent >>>>>> > > module, and only bundle within the ORM main jar as final >>>>>> distribution >>>>>> > > step. >>>>>> > > >>>>>> > > If that's too much work, I'd rather make the geolatte-geom a >>>>>> mandatory >>>>>> > > dependency than to have cryptic runtime failures. >>>>>> > > >>>>>> > > On 27 June 2016 at 12:41, Vlad Mihalcea <mihalcea.v...@gmail.com> >>>>>> wrote: >>>>>> > > > Hi, >>>>>> > > > >>>>>> > > > Since hibenrate-spatial has been merged into Hibernate code >>>>>> base, >>>>>> > > shouldn't >>>>>> > > > we merge the Dialects as well. >>>>>> > > > For instance, we have MySQL56InnoDBSpatialDialect which can >>>>>> simply be >>>>>> > > > merged into a MySQL56InnoDBDialect. >>>>>> > > > This way, MySQL57InnoDBDialect can take advantage of spatial >>>>>> queries as >>>>>> > > > well. >>>>>> > > > >>>>>> > > > The only drawback is that we need to add the geolatte-geom lib >>>>>> to >>>>>> > > > hibernate-orm. >>>>>> > > > >>>>>> > > > Let me know what you think. >>>>>> > > > >>>>>> > > > Vlad >>>>>> > > > _______________________________________________ >>>>>> > > > hibernate-dev mailing list >>>>>> > > > hibernate-dev@lists.jboss.org >>>>>> > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> > > _______________________________________________ >>>>>> > > hibernate-dev mailing list >>>>>> > > hibernate-dev@lists.jboss.org >>>>>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> > > >>>>>> > _______________________________________________ >>>>>> > hibernate-dev mailing list >>>>>> > hibernate-dev@lists.jboss.org >>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> > >>>>>> _______________________________________________ >>>>>> hibernate-dev mailing list >>>>>> hibernate-dev@lists.jboss.org >>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> >>>>> >>>>> >>> > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev