[hibernate-dev] Patch for HHH-272: Custom SQL for column gets and sets

[Steve Ebersole]

Do you use IRC at all?  We could chat there.

On Sat, 2009-09-05 at 09:21 -0400, Rob Hasselbaum wrote:
> 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 
> > 
> 
-- 
Steve Ebersole <steve at hibernate.org>
Hibernate.org