I've submitted a patch with unit tests for HHH-272 and I'm interested in feedback.

Awesome! This is one of those things I always wanted to get implemented but it was always getting pushed down my priority list. Thanks for taking on that work. I'll definitely take a look this week. If you could work it up for trunk that would be better. New features should really not get added to maintenance releases. On Wed, 2009-09-02 at 17:17 -0400, Rob Hasselbaum wrote: > Hi, > > I've submitted a patch with unit tests for HHH-272 and I'm interested > in feedback. The patch allows users to specify custom SQL get and set > expressions for property columns. This can be used to call SQL > functions or perform some other kind of database-side conversion > without giving up automatic statement generation. You use it like > this: > > <property name="creditCardNumber"> > <column name="credit_card_num" sql-set="encrypt(?)" > sql-get="decrypt(credit_card_num)"/> > </property> > > This is similar to a formula property with two differences. First, the > data is backed by a real column, which gets exported with the schema. > And second. the data is read-write, not read-only. The sql-get > expression gets applied in predicates, order clauses, projections, > etc. > > The patch should work for all property columns including those in > components and composite-elements. I didn't try to get id, key, index, > or other kinds of non-property columns to work. It is built on top of > the 3.3.2.GA tag, but I can port it to trunk if necessary. I also have > a patch nearly ready to expose this functionality in Hibernate > Annotations in case the Core patch is accepted. If any commiters could > have a look, I'd appreciate it. > > Thanks for your time, > -Rob Hasselbaum > _______________________________________________ > hibernate-dev mailing list > hibernate-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev

Hi Steve, Thanks for the feedback. I am working on moving the patch to trunk as you requested. I'll update the bug when that's done. My comments on the rest of your feedback are inline below. On 09/03/2009 04:39 PM, Steve Ebersole wrote: > 1) Personally, I don't like the attribute names sql-get and sql-set. > When I think through trying to describe and explain this feature to > people, the terms "wrap" and "unwrap" keep coming to my head as being > descriptive, relevant and natural. It was really the "naturalness" > aspect that got me with sql-get and sql-set. Other terms I could think > of included encode/decodeAny other suggestions? I went with "sql-set" and "sql-get" because they are consistent with the existing "sql-type" attribute and make sense to me, but I'm not married to the terms. Wrap and unwrap sound good, too. Bear in mind that the expressions need not be function calls, which wrap and unwrap might suggest. The functional tests do math, for instance. > 2) Can anyone foresee a valid use-case for allowing one of these to be > defined w/o the other? The only thing I came up with was for immutable > properties. So something like <column name="xyz" unwrap="decode(xyz)" > insert="false" update="false"/>, which can also be defined using a > formula like <formula>decode(xyz)</formula>. The reason I ask is that > if they should always be used together then maybe it is better to > enforce that in the DTD and/or binder > I couldn't think of a use case for a one-way conversion either. In fact, you could end up reading out a different value than the one you put in if you're not careful. But I'd regret adding an artificial restriction if someone came up with a legitimate use case next year. > 3) You renamed the getTemplate method on Selectable to > getGetterTemplate. Everything in the o.h.mapping package is an SPI that > is used by many other libraries. We need to be very careful about > changing stuff in here. I am not sure if folks bind to this particular > method. But since this is just a cosmetic change, I think it should be > reverted. > > Sorry, this was an oversight. At first, I thought I was going to need to store a setter template in addition to a getter template in the Column class, It turned out that I didn't need it, but I forgot to revert the name of the getter template. I'll fix that. > Still not done looking through the whole patch. Will try to finish up > tomorrow. > > Thanks, -Rob

What is this doing that a parameterized custom type can't do ? Is this "just" to get a cleaner hbm.xml syntax or ? /max

Steve, Any additional input before I upload a new patch? -Rob On 09/03/2009 10:28 PM, Rob Hasselbaum wrote: > Hi Steve, > > Thanks for the feedback. I am working on moving the patch to trunk > as you requested. I'll update the bug when that's done. My comments > on the rest of your feedback are inline below. > > On 09/03/2009 04:39 PM, Steve Ebersole wrote: > > 1) Personally, I don't like the attribute names sql-get and > > sql-set. > > When I think through trying to describe and explain this feature > > to > > people, the terms "wrap" and "unwrap" keep coming to my head as > > being > > descriptive, relevant and natural. It was really the > > "naturalness" > > aspect that got me with sql-get and sql-set. Other terms I could > > think > > of included encode/decodeAny other suggestions? > > I went with "sql-set" and "sql-get" because they are consistent with > the existing "sql-type" attribute and make sense to me, but I'm not > married to the terms. Wrap and unwrap sound good, too. Bear in mind > that the expressions need not be function calls, which wrap and > unwrap might suggest. The functional tests do math, for instance. > > > 2) Can anyone foresee a valid use-case for allowing one of these > > to be > > defined w/o the other? The only thing I came up with was for > > immutable > > properties. So something like <column name="xyz" > > unwrap="decode(xyz)" > > insert="false" update="false"/>, which can also be defined using > > a > > formula like <formula>decode(xyz)</formula>. The reason I ask is > > that > > if they should always be used together then maybe it is better to > > enforce that in the DTD and/or binder > > > > I couldn't think of a use case for a one-way conversion either. In > fact, you could end up reading out a different value than the one > you put in if you're not careful. But I'd regret adding an > artificial restriction if someone came up with a legitimate use case > next year. > > > 3) You renamed the getTemplate method on Selectable to > > getGetterTemplate. Everything in the o.h.mapping package is an > > SPI that > > is used by many other libraries. We need to be very careful > > about > > changing stuff in here. I am not sure if folks bind to this > > particular > > method. But since this is just a cosmetic change, I think it > > should be > > reverted. > > > > > > Sorry, this was an oversight. At first, I thought I was going to > need to store a setter template in addition to a getter template in > the Column class, It turned out that I didn't need it, but I forgot > to revert the name of the getter template. I'll fix that. > > > Still not done looking through the whole patch. Will try to > > finish up > > tomorrow. > > > > > > Thanks, > -Rob >