Re: [hibernate-dev] Envers compile issue

Hi Lukasz, The problems seem limited to usages of: 1) org.hibernate.envers.configuration.internal.metadata.MetadataTools#getColumnNameIterator 2) org.hibernate.envers.configuration.internal.metadata.MetadataTools#addColumns from: 1) org.hibernate.envers.configuration.internal.metadata.AuditMetadataGenerator#createJoins 2) org.hibernate.envers.configuration.internal.metadata.BasicMetadataGenerator#addManyToOne 3) org.hibernate.envers.configuration.internal.metadata.BasicMetadataGenerator#addBasic 4) org.hibernate.envers.configuration.internal.metadata.CollectionMetadataGenerator#addWithMiddleTable 5) org.hibernate.envers.configuration.internal.metadata.CollectionMetadataGenerator#addValueToMiddleTable 6) org.hibernate.envers.configuration.internal.metadata.ToOneRelationMetadataGenerator#addToOne org.hibernate.envers.configuration.internal.metadata.MetadataTools#addColumns unfortunately goes through a number of internal addColumns/addColumn calls. Oddly enough, I found that there is in fact: 1) org.hibernate.envers.configuration.internal.metadata.MetadataTools#addColumnsOrFormulas 2) org.hibernate.envers.configuration.internal.metadata.MetadataTools#addFormula But that is only ever used to handle discriminators, as you mentioned. Additionally, I then text searched the envers testsuite and found that only the org.hibernate.envers.test.integration.inheritance.single.discriminatorformula package contained any references to the text 'formula', which further illustrates that. So, all that said, I *think* envers already errored when it encountered formula (with CCE). In the code I saw it is clearly expecting Iterator<Column> and not even checking if the Iterator returned Formula elements mixed in. At the end of the day it is still y'alls call. Based on my reading of the current code, I think a comparable (though better) solution is to explicitly check each Iterator element type and throw that more descriptive exception[1]. That change I can make. If y'all would rather add in real formula support, thats probably something y'all will have to work on. WDYT? [1] For example, FormulaNotSupportException( "Formula mappings are currently not supported in envers aside from @DiscriminatorValue; for more information, please see blah-blah-blah..." ) On Apr 12, 2013 4:52 AM, "Łukasz Antoniak" <lukasz.antoniak(a)gmail.com <mailto:lukasz.antoniak@gmail.com>> wrote: Hello Steve, In general Envers does not support formula columns. Probably we should save formula value to audit table. This can be tricky and I have to think it over. Copying formula column to audit entity would not produce expected result in all cases (e.g. time functions used in formula expression). Currently we support @DiscriminatorFormula annotation without saving its value to audit table, since entities do not change their type. @Adam: Thoughts? What do you think about copying formula column to audit entity directly? Simplest solution AFAIK. All usages of org.hibernate.mapping.Value#getColumnIterator(), except the one in org.hibernate.envers.configuration.internal.metadata.BasicMetadataGenerator, map relation properties. IMO it is safe assumption that we are dealing with columns there. @Steve: if you agree, feel free to throw an exception if encountered formula column in BasicMetadataGenerator asking to register new feature. Best regards, Lukasz 2013/4/11 Steve Ebersole <steve(a)hibernate.org <mailto:steve@hibernate.org>> One option just came to mind. Looking at the code, I have to assume envers really just does not support formulas. Otherwise, all these compile errors would eventually have resulted in runtime (CCE) exceptions. If that is really the case, I could add detection of that and throw a more meaningful exception. If that sounds like a reasonable option, just let me know and I'll make that change. Alternatively I could just log a message and keep going, but i don't think that's a good choice. On Thu 11 Apr 2013 04:07:27 PM CDT, Steve Ebersole wrote: I just made a change to help catch runtime problems that kept cropping up. The change was to org.hibernate.mapping.Value#getColumnIterator. The problem is that code in many modules (hem, envers) that actually deals with mapping code were making a bad assumption here. The returned iterator actually returns a Iterable<Selectable>, not Iterable<Column>. Selectable is the contract shared between Column and Formula. So when code non-defensively tries to treat that thing as a Iterable<Column> we often have issues. Yes, the method is very poorly named. Actually it predates formula as a feature. But be that as it may, the code casting those elements to Column are just wrong. For envers, I am not actually sure how to handle Formula elements. I need some help there. The issues are all isolated to org.hibernate.envers.configuration.internal.metadata Could someone more familiar with envers (and especially its entity definition stuff) take a look? Thanks!