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.v...@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, <cranc...@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.bei...@gmail.com <mailto:christian.bei...@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 > > >> <st...@hibernate.org <mailto:st...@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.v...@gmail.com <mailto:mihalcea.v...@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 > > >> <st...@hibernate.org <mailto:st...@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.v...@gmail.com <mailto:mihalcea.v...@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@lists.jboss.org > > >> <mailto:hibernate-dev@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@lists.jboss.org <mailto: > > hibernate-dev@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@lists.jboss.org <mailto: > hibernate-dev@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@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