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.vlad(a)gmail.com&gt; 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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

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 <steve(a)hibernate.org&gt; 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 <sanne(a)hibernate.org&gt; 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.vlad(a)gmail.com&gt; 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(a)lists.jboss.org > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > _______________________________________________ > > hibernate-dev mailing list > > hibernate-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

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 <gunnar(a)hibernate.org&gt; 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 <karel(a)geovise.com&gt;: > >> 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 <steve(a)hibernate.org&gt; >> 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 <sanne(a)hibernate.org&gt; >> 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.vlad(a)gmail.com&gt; >> 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(a)lists.jboss.org >> > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > > _______________________________________________ >> > > hibernate-dev mailing list >> > > hibernate-dev(a)lists.jboss.org >> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > > >> > _______________________________________________ >> > hibernate-dev mailing list >> > hibernate-dev(a)lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >> _______________________________________________ >> hibernate-dev mailing list >> hibernate-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > >

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 <karel(a)geovise.com&gt; 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 <steve(a)hibernate.org&gt; > 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 <gunnar(a)hibernate.org&gt; >> 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 <karel(a)geovise.com&gt;: >>> >>>> 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 <steve(a)hibernate.org&gt; >>>> 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 <sanne(a)hibernate.org&gt; >>>> 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.vlad(a)gmail.com&gt; >>>> 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(a)lists.jboss.org >>>> > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> > > _______________________________________________ >>>> > > hibernate-dev mailing list >>>> > > hibernate-dev(a)lists.jboss.org >>>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> > > >>>> > _______________________________________________ >>>> > hibernate-dev mailing list >>>> > hibernate-dev(a)lists.jboss.org >>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> > >>>> _______________________________________________ >>>> hibernate-dev mailing list >>>> hibernate-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>> >>> >>> >

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 <karel(a)geovise.com&gt; 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 <steve(a)hibernate.org&gt; > 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 <karel(a)geovise.com&gt; 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 <steve(a)hibernate.org&gt; >>> 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 <gunnar(a)hibernate.org&gt; >>>> 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 <karel(a)geovise.com&gt;: >>>>> >>>>>> 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 <steve(a)hibernate.org&gt; >>>>>> 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 <sanne(a)hibernate.org&gt; >>>>>> 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.vlad(a)gmail.com&gt; >>>>>> 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(a)lists.jboss.org >>>>>> > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> > > _______________________________________________ >>>>>> > > hibernate-dev mailing list >>>>>> > > hibernate-dev(a)lists.jboss.org >>>>>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> > > >>>>>> > _______________________________________________ >>>>>> > hibernate-dev mailing list >>>>>> > hibernate-dev(a)lists.jboss.org >>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> > >>>>>> _______________________________________________ >>>>>> hibernate-dev mailing list >>>>>> hibernate-dev(a)lists.jboss.org >>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>>> >>>>> >>>>> >>> > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev