[hibernate-dev] Merging hibernate spatial Dialects with the core ones

Tue Jun 28 16:07:23 EDT 2016

On 28 June 2016 at 18:43, Steve Ebersole <steve at hibernate.org> wrote:
> 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.

+1
I agree, my previous comment was mislead as I thought it was merged
already, I didn't mean to suggest merging it.
Better keep them separated.


>
> On Tue, Jun 28, 2016, 11:02 AM Karel Maesen <karel at 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 <steve at 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 <karel at 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 <steve at 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 <gunnar at 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 <karel at 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 <steve at 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 <sanne at 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.vlad at 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 at lists.jboss.org
>>>>>>> > > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>> > > _______________________________________________
>>>>>>> > > hibernate-dev mailing list
>>>>>>> > > hibernate-dev at lists.jboss.org
>>>>>>> > > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>> > >
>>>>>>> > _______________________________________________
>>>>>>> > hibernate-dev mailing list
>>>>>>> > hibernate-dev at lists.jboss.org
>>>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>> >
>>>>>>> _______________________________________________
>>>>>>> hibernate-dev mailing list
>>>>>>> hibernate-dev at lists.jboss.org
>>>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev