[hibernate-dev] Simplify SQL function registration when bootstrapping via JPA

Steve Ebersole steve at hibernate.org
Fri May 18 08:05:36 EDT 2018 

There could conceivably be multiple MetadataBuilderContributor instances
needing to be integrated.  CDI (and therefore ManagedBeanRegistry) do not
work well in this situation.  MetadataBuilderContributor should be done via
ServiceLoader (via ClassLoaderService)

When I mentioned ManagedBeanRegistry, I was talking specifically about just
contributing functions - but MetadataBuilderContributor is more
generalized, which is fine.. it just won't fit into the CDI paradigm




On Thu, May 17, 2018 at 1:35 PM Vlad Mihalcea <mihalcea.vlad at gmail.com>
wrote:

> I named it MetadataBuilderContributor because you can do more than just
> registering SqlFunctions. While implementing it, I realized that, this way,
> it's going to be very flexible to customize the Hibernate-native Metadata
> while doing the JPA bootstrap.
>
> Related to ManagedBeanRegitry, I'm trying to convert to using it, but
> stumbled on this issue.
>
> If I try to get a Bean like this:
>
> managedBeanRegistry.getBean( MetadataBuilderContributor.class );
>
> And there is no BeanContainer set via
> "hibernate.resource.beans.container", then the FallbackContainedBean is
> used, which tries to instantiate the provided interface:
>
> beanType.newInstance();
>
> But since I provide an interface, the call will fail.
>
> The only workaround I found is this:
>
> ManagedBeanRegistry managedBeanRegistry = standardServiceRegistry
> .getService( ManagedBeanRegistry.class );
>
> BeanContainer beanContainer = managedBeanRegistry.getBeanContainer();
>
> ManagedBean<MetadataBuilderContributor>
> metadataBuilderContributorManagedBean = null;
> if ( beanContainer != null ) {
> metadataBuilderContributorManagedBean = managedBeanRegistry
> .getBean( MetadataBuilderContributor.class );
> }
>
> MetadataBuilderContributor metadataBuilderContributor =
> metadataBuilderContributorManagedBean
> .getBeanInstance();
>
> Am I using it the wrong way, or do we need to check the BeanContainer
> first to make sure it's not null before getting a Bean from the
> ManagedBeanRegistry?
>
> Vlad
>
>
> On Thu, May 17, 2018 at 9:07 PM, Steve Ebersole <steve at hibernate.org>
> wrote:
>
>> Personally I think both a contributor and CDI integration
>> (ManagedBeanRegitry) make sense here.  Btw, the name for the contributor
>> should not be SqlFunctionContributor - it should reflect the ultimate goal
>> here which is SqmFunctionTemplate; I can see SqmFunctionContributor or just
>> FunctionContributor.  This is an example of what I meant about waiting for
>> 6...
>>
>> On Thu, May 17, 2018 at 12:50 PM Vlad Mihalcea <mihalcea.vlad at gmail.com>
>> wrote:
>>
>>> Sure thing. I'll try to adapt it to using the Bean registry.
>>>
>>> Vlad
>>>
>>> On Thu, 17 May 2018, 20:07 Chris Cranford, <crancran at gmail.com> wrote:
>>>
>>> > I personally agree with Christian, I don't see the use of the
>>> > ManagedBeanRegistry being a problem.  The new interface certainly opens
>>> > the door for a variety of builder settings to be contributed easily and
>>> > using the registry allows for a versatile way to provide that bean,
>>> > whether it be through some CDI/Spring environment or the fallback
>>> > solution where we instantiate it ourselves.  I believe the code as you
>>> > have it can easily be adapted to use the registry by replacing the
>>> > "newInstance()" call with a call into getting the bean from the
>>> > registry.  The registry itself internally should handle getting the
>>> bean
>>> > from the container or returning you a new instance we instantiate
>>> > ourselves.
>>> >
>>> > On 05/17/2018 12:25 PM, Christian Beikov wrote:
>>> > > The functions could be contributed via a CDI/Spring bean which might
>>> not
>>> > > be such a bad idea I think. In a test environment there could be a
>>> > > different contributor active than in production. Of course, this
>>> could
>>> > > also be handled by passing in different configuration property
>>> values,
>>> > > but that's why I was saying I'm not sure about it. Maybe someone else
>>> > > has an idea if using ManagedBeanRegistry might make sense?
>>> > >
>>> > >
>>> > > Mit freundlichen Grüßen,
>>> > >
>>> ------------------------------------------------------------------------
>>> > > *Christian Beikov*
>>> > > Am 17.05.2018 um 16:49 schrieb Vlad Mihalcea:
>>> > >> Hi,
>>> > >>
>>> > >> Looking at the SessionFactoryImpl class, the only way to provide an
>>> > >> SqlFunction is either via the Dialect or via the
>>> SessionFactoryOptions:
>>> > >> this.sqlFunctionRegistry  =new SQLFunctionRegistry(
>>> > jdbcServices.getJdbcEnvironment().getDialect(),
>>> > options.getCustomSqlFunctionMap() );
>>> > >> I'm not sure if the ManagedBeanRegistry would have helped in this
>>> case
>>> > >> without requiring more code changes.
>>> > >>
>>> > >> Vlad
>>> > >>
>>> > >> On Thu, May 17, 2018 at 5:22 PM, Christian Beikov
>>> > >> <christian.beikov at gmail.com <mailto:christian.beikov at gmail.com>>
>>> wrote:
>>> > >>
>>> > >>     It looks ok to me although I'm not sure if it wouldn't be more
>>> > >>     appropriate to instantiate the contributor via
>>> ManagedBeanRegistry.
>>> > >>
>>> > >>
>>> > >>     Mit freundlichen Grüßen,
>>> > >>
>>> >
>>> ------------------------------------------------------------------------
>>> > >>     *Christian Beikov*
>>> > >>     Am 17.05.2018 um 11:20 schrieb Vlad Mihalcea:
>>> > >>     > In the end, I thought of a more generic approach to for JPA
>>> > >>     bootstrapping
>>> > >>     > which, not only allows us to register SqlFunctions, but we can
>>> > >>     apply other
>>> > >>     > changes on the MetadataBuilder object used during bootstrap.
>>> > >>     >
>>> > >>     > This is the Pull Request:
>>> > >>     >
>>> > >>     > https://github.com/hibernate/hibernate-orm/pull/2288
>>> > >>     <https://github.com/hibernate/hibernate-orm/pull/2288>
>>> > >>     >
>>> > >>     > Let me know what you think.
>>> > >>     >
>>> > >>     > Vlad
>>> > >>     >
>>> > >>     > On Wed, May 16, 2018 at 3:55 PM, Steve Ebersole
>>> > >>     <steve at hibernate.org <mailto:steve at hibernate.org>> wrote:
>>> > >>     >
>>> > >>     >> Its not so much hindering 6.0 that I am concerned with.  The
>>> > >>     problem is
>>> > >>     >> updatability for the user.  The more things they use that
>>> > >>     change = the more
>>> > >>     >> work to upgrade.
>>> > >>     >>
>>> > >>     >> On Wed, May 16, 2018 at 6:51 AM Vlad Mihalcea
>>> > >>     <mihalcea.vlad at gmail.com <mailto:mihalcea.vlad at gmail.com>>
>>> > >>     >> wrote:
>>> > >>     >>
>>> > >>     >>> I suppose this will be refactored considerably in 6.x.
>>> > >>     >>>
>>> > >>     >>> However, it's just a small change and I don't think it will
>>> > >>     hinder any
>>> > >>     >>> 6.x changes.
>>> > >>     >>>
>>> > >>     >>> I'm thinking of defining an SqlFunctionContributor interface
>>> > >>     >>> (@FunctionalInterface)
>>> > >>     >>> which can be provided via the properties Map that's supplied
>>> > >>     when using
>>> > >>     >>> the Persistence#createEntityManagerFactory method.
>>> > >>     >>>
>>> > >>     >>> In EntityManagerFactoryBuilder, I can just inspect that and
>>> > >>     pass it
>>> > >>     >>> further to MetamodelBuilder.
>>> > >>     >>>
>>> > >>     >>> So, it does not take any effort to implement it and users
>>> can
>>> > >>     benefit
>>> > >>     >>> from it. I recently answered a question on the forum on this
>>> > >>     topic:
>>> > >>     >>>
>>> > >>     >>> https://discourse.hibernate.org/t/i-cant-use-group-concat-
>>> > >>     <https://discourse.hibernate.org/t/i-cant-use-group-concat->
>>> > >>     >>> in-queries/752/14
>>> > >>     >>>
>>> > >>     >>> And, realized that I was wrong about suggesting doing it
>>> via the
>>> > >>     >>> Integrator mechanism (since the Metamodel is already frozen
>>> by
>>> > >>     the time we
>>> > >>     >>> execute the Integrator).
>>> > >>     >>>
>>> > >>     >>> Vlad
>>> > >>     >>>
>>> > >>     >>> On Wed, May 16, 2018 at 2:41 PM, Steve Ebersole
>>> > >>     <steve at hibernate.org <mailto:steve at hibernate.org>>
>>> > >>     >>> wrote:
>>> > >>     >>>
>>> > >>     >>>> This should maybe wait for 6.0.  We are ditching
>>> SQLFunction
>>> > >>     in favor of
>>> > >>     >>>> a more AST-friendly contract.
>>> > >>     >>>>
>>> > >>     >>>> Beyond that, go for it.
>>> > >>     >>>>
>>> > >>     >>>>
>>> > >>     >>>> On Wed, May 16, 2018, 6:34 AM Vlad Mihalcea
>>> > >>     <mihalcea.vlad at gmail.com <mailto:mihalcea.vlad at gmail.com>>
>>> > >>     >>>> wrote:
>>> > >>     >>>>
>>> > >>     >>>>> Hi,
>>> > >>     >>>>>
>>> > >>     >>>>> I realized that only the native Hibernate bootstrapping
>>> > >>     mechanism allows
>>> > >>     >>>>> for passing custom SQL functions.
>>> > >>     >>>>>
>>> > >>     >>>>> For JPA, we can extend the Dialect to register, but that's
>>> > >>     not always
>>> > >>     >>>>> desirable as it requires a code change
>>> > >>     >>>>> every time the user switches to a new Dialect version.
>>> > >>     >>>>>
>>> > >>     >>>>> For this reason, I created this Jira issue:
>>> > >>     >>>>>
>>> > >>     >>>>> https://hibernate.atlassian.net/browse/HHH-12589
>>> > >>     <https://hibernate.atlassian.net/browse/HHH-12589>
>>> > >>     >>>>>
>>> > >>     >>>>> Let me know if anyone has anything against implementing
>>> this
>>> > >>     feature.
>>> > >>     >>>>>
>>> > >>     >>>>> Vlad
>>> > >>     >>>>> _______________________________________________
>>> > >>     >>>>> hibernate-dev mailing list
>>> > >>     >>>>> hibernate-dev at lists.jboss.org
>>> > >>     <mailto:hibernate-dev at lists.jboss.org>
>>> > >>     >>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>> > >>     <https://lists.jboss.org/mailman/listinfo/hibernate-dev>
>>> > >>     >>>>>
>>> > >>     > _______________________________________________
>>> > >>     > hibernate-dev mailing list
>>> > >>     > hibernate-dev at lists.jboss.org <mailto:
>>> > hibernate-dev at lists.jboss.org>
>>> > >>     > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>> > >>     <https://lists.jboss.org/mailman/listinfo/hibernate-dev>
>>> > >>
>>> > >>     _______________________________________________
>>> > >>     hibernate-dev mailing list
>>> > >>     hibernate-dev at lists.jboss.org <mailto:
>>> hibernate-dev at lists.jboss.org
>>> > >
>>> > >>     https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>> > >>     <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
>>
>>
>

More information about the hibernate-dev mailing list