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

Tue Jun 28 13:43:15 EDT 2016

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 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
>>>>>>
>>>>>
>>>>>
>>>
>