Re: [hibernate-dev] javax.money
It's software, so everything is *possible*. Question is whether we deem it acceptable for the user having to depend on another submodule or not. If you think that's ok, then go for it ;) 2015-12-22 23:09 GMT+01:00 Steve Ebersole : > You mean in addition to Maven's own documentation? Quote[1]: > > "Optional dependencies are used when it's not really possible (for whatever > reason) to split a project up into sub-modules. The idea is that some of the > dependencies are only used for certain features in the project, and will not > be needed if that feature isn't used. Ideally, such a feature would be split > into a sub-module that depended on the core functionality project...this new > subproject would have only non-optional dependencies, since you'd need them > all if you decided to use the subproject's functionality." > > Beyond Maven's own documentation, simply google... > > [1] > https://maven.apache.org/guides/introduction/introduction-to-optional-and-excludes-dependencies.html > > On Tue, Dec 22, 2015 at 3:50 PM Gunnar Morling wrote: >> >> To me, "optional" seems like the right thing for this case. "Provided" >> (at least in the Maven world, I don't know whether Gradle has >> different semantics) suggests that this stuff is really to be provided >> by the runtime; I interpret this as a mandatory requirement towards >> the environment (i.e. a Java EE container *has* to provide the javax.* >> packages). >> >> Optional instead indicates that this stuff may be present at runtime >> or not. If it is present, some optional functionality will be >> available to the user, otherwise not. For the case of Money the user >> is depending on these types anyways - and thus required to expose them >> on the runtime class path - if he is wishing to work with these types >> in his own code (or not otherwise). No need to analyse the dependency >> graph or whatsoever. >> >> As said we use that style in Hibernate Validator for optional >> constraint validators, and it works well, I can't remember of any >> related issue or complaint. I suppose it'd help if you could point to >> that resource describing potential issues with optional. >> >> 2015-12-22 20:31 GMT+01:00 Steve Ebersole : >> > Well the Bitronix use-case is actually a perfect illustration of why >> > optional/provided dependencies stink. Nothing in the dependency graph >> > indicates what you are expected to do here. Bitronix users would have >> > to >> > look at the documentation to understand this; and dependency >> > graph/analysis >> > tools would be completely at a loss. And in Bitronix the "decision" is >> > all >> > internal to Bitronix. Here at least the argument is a little more >> > justified because we are really talking about looking to see if another >> > set >> > of classes (library) are present from the application's classloader >> > (since >> > the application would need to bind to Moneta, etc statically). >> > >> > 'optional' is flat out wrong; that is well documented elsewhere. I can >> > be >> > persuaded to use 'provided' for something like this, as I already >> > stated. >> > >> > >> > On Tue, Dec 22, 2015 at 12:26 PM Vlad Mihalcea >> > wrote: >> > >> >> That's how Bitronix Transaction Manager managed optional dependencies >> >> as >> >> well. >> >> In its case cglib and javassist were optional dependencies and at >> >> runtime >> >> the framework decided whether to load one provider or the other. >> >> >> >> Vlad >> >> >> >> On Tue, Dec 22, 2015 at 8:01 PM, Sanne Grinovero >> >> wrote: >> >> >> >> > On 22 December 2015 at 17:34, Gunnar Morling >> >> wrote: >> >> > >> yes we could use an optional/provided dependency, but >> >> > >> that would not be "proper". >> >> > > >> >> > > That would actually be my preference; If the javax.money types are >> >> > > present, enable the required Types etc. otherwise not. It's a >> >> > > pattern >> >> > > we e.g. use in Validator, too, where we provide additional >> >> > > constraint >> >> > > validator types based on what's available in the runtime >> >> > > environment. >> >> > >> >> > +1 >> >> > >> >> > > Why do you consider it as not proper? >> >> > >> >> > Same question for me. It might not be perfectly well defined in terms >> >> > of Maven dependencies, still I think the practical benefit for users >> >> > far outweights the cons.. if that's what you're referring to. >> >> > >> >> > Sanne >> >> > >> >> > > >> >> > > >> >> > > 2015-12-22 17:03 GMT+01:00 Steve Ebersole : >> >> > >> I had planned to support this JSR when I thought it would be part >> >> > >> of >> >> the >> >> > >> JDK. But as it is, supporting this properly would mean a whole >> >> > >> new >> >> > >> module/artifact just to add these few types + the dependency >> >> > >> (similar >> >> to >> >> > >> hibernate-java8). I am not a fan of the fact that it would >> >> > >> require a >> >> > >> separate dependency; yes we could use an optional/provided >> >> > >> dependency, >> >> > but >> >> > >> that would not be "proper".
[hibernate-dev] HHH-5855 bug report and a possible fix
Hi guys
I decided to spend some time to investigate the infamous HHH-5855 (
https://hibernate.atlassian.net/browse/HHH-5855 ) bug and this is my
report.
One of the first thing that I noticed is that Sets are fine, while this bug
only replicates with bidirectional Bags.
After some hours of debugging and logging (since debugging triggers
collection initialization), I found the culprit.
In the org.hibernate.type.TypeHelper.replace(Object[] original, Object[]
target, Type[] types, SessionImplementor session, Object owner, Map
copyCache) method, when copying the cached entity state (which contains the
newly added child entity along with its identifier) onto the original
collection:
copied[i] = types[i].replace( original[i], target[i], session, owner,
copyCache );
inside the org.hibernate.type.CollectionType.replace(Object[] original,
Object[] target, Type[] types, SessionImplementor session, Object owner,
Map copyCache) method there is this check:
if ( !Hibernate.isInitialized( original ) ) {
return target;
}
For Sets, the collection is always initialized because of this line inside
the add() method of the org.hibernate.collection.internal.PersistentSet:
final Boolean exists = isOperationQueueEnabled() ?
readElementExistence( value ) : null;
Because of the the readElementExistence( value ) call the Set is always
initialized and upon triggering the flush, the newly added Entity being
already managed it will be left alone.
For PersistentList the aforementioned check is false and the replace never
occurs, hence the transient entity lingers in the persistence context and
the flush will trigger another insert, so we get duplicates.
To make the Bag behave like the Set, all we need to do is to change the add
method like this:
public boolean add(Object object) {
initialize(false);
if ( !isOperationQueueEnabled() ) {
write();
return bag.add( object );
}
else {
queueOperation( new SimpleAdd( object ) );
return true;
}
}
but then four tests will fail:
org.hibernate.test.legacy.MasterDetailTest > testQueuedBagAdds FAILED
java.lang.AssertionError at MasterDetailTest.java:1068
org.hibernate.test.unionsubclass.UnionSubclassTest > testUnionSubclass
FAILED
org.hibernate.ObjectDeletedException at UnionSubclassTest.java:364
org.hibernate.test.version.VersionTest > testCollectionNoVersion FAILED
java.lang.AssertionError at VersionTest.java:118
org.hibernate.test.version.VersionTest > testCollectionVersion FAILED
java.lang.AssertionError at VersionTest.java:79
3 of them fail because we expect the List not to be initialized and
the UnionSubclassTest fails
for a doubtful reason (we attempt to delete an entity which is still
referenced).
Basically, such a change will finally fix this issue and the Sets and Lists
will behave consistently. Since you know the reasons behind the difference
in how Sets and Lists are initialized, we need to discuss if this change is
appropriate or we should address this issue differently.
I have a branch on my fork with a test that replicates this issue and that
the proposed change manages to fix it:
https://github.com/vladmihalcea/hibernate-orm/tree/feature/hhh5855
Let me know what you think and let's discuss it further.
Vlad
___
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] HHH-5855 bug report and a possible fix
Hi Vlad,
I've spend quite a bit of time on this one and already have a fix. I just
have some tests to add to confirm. I will look into what you suggest, but
please check with me first if you see that an issue is assigned to me.
Thanks,
Gail
On Wed, Dec 23, 2015 at 4:13 AM, Vlad Mihalcea
wrote:
> Hi guys
>
> I decided to spend some time to investigate the infamous HHH-5855 (
> https://hibernate.atlassian.net/browse/HHH-5855 ) bug and this is my
> report.
>
> One of the first thing that I noticed is that Sets are fine, while this bug
> only replicates with bidirectional Bags.
>
> After some hours of debugging and logging (since debugging triggers
> collection initialization), I found the culprit.
>
> In the org.hibernate.type.TypeHelper.replace(Object[] original, Object[]
> target, Type[] types, SessionImplementor session, Object owner, Map
> copyCache) method, when copying the cached entity state (which contains the
> newly added child entity along with its identifier) onto the original
> collection:
>
> copied[i] = types[i].replace( original[i], target[i], session, owner,
> copyCache );
>
> inside the org.hibernate.type.CollectionType.replace(Object[] original,
> Object[] target, Type[] types, SessionImplementor session, Object owner,
> Map copyCache) method there is this check:
>
> if ( !Hibernate.isInitialized( original ) ) {
>return target;
> }
>
>
> For Sets, the collection is always initialized because of this line inside
> the add() method of the org.hibernate.collection.internal.PersistentSet:
>
> final Boolean exists = isOperationQueueEnabled() ?
> readElementExistence( value ) : null;
>
> Because of the the readElementExistence( value ) call the Set is always
> initialized and upon triggering the flush, the newly added Entity being
> already managed it will be left alone.
>
> For PersistentList the aforementioned check is false and the replace never
> occurs, hence the transient entity lingers in the persistence context and
> the flush will trigger another insert, so we get duplicates.
>
> To make the Bag behave like the Set, all we need to do is to change the add
> method like this:
>
> public boolean add(Object object) {
>initialize(false);
>if ( !isOperationQueueEnabled() ) {
> write();
> return bag.add( object );
>}
>else {
> queueOperation( new SimpleAdd( object ) );
> return true;
>}
> }
>
> but then four tests will fail:
>
> org.hibernate.test.legacy.MasterDetailTest > testQueuedBagAdds FAILED
> java.lang.AssertionError at MasterDetailTest.java:1068
>
> org.hibernate.test.unionsubclass.UnionSubclassTest > testUnionSubclass
> FAILED
> org.hibernate.ObjectDeletedException at UnionSubclassTest.java:364
>
> org.hibernate.test.version.VersionTest > testCollectionNoVersion FAILED
> java.lang.AssertionError at VersionTest.java:118
>
> org.hibernate.test.version.VersionTest > testCollectionVersion FAILED
> java.lang.AssertionError at VersionTest.java:79
>
> 3 of them fail because we expect the List not to be initialized and
> the UnionSubclassTest fails
> for a doubtful reason (we attempt to delete an entity which is still
> referenced).
>
> Basically, such a change will finally fix this issue and the Sets and Lists
> will behave consistently. Since you know the reasons behind the difference
> in how Sets and Lists are initialized, we need to discuss if this change is
> appropriate or we should address this issue differently.
>
> I have a branch on my fork with a test that replicates this issue and that
> the proposed change manages to fix it:
>
> https://github.com/vladmihalcea/hibernate-orm/tree/feature/hhh5855
>
> Let me know what you think and let's discuss it further.
>
> Vlad
> ___
> 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
Re: [hibernate-dev] HHH-5855 bug report and a possible fix
We really don't want to initialize a List when merging. Instead, we want to
do the same sort of replace on the values stored in the DelayedOperation
objects. That way, the collection will be initialized only when necessary.
The DelayedOperations are executed on flush. I'll should get a pull request
out for this today or tomorrow.
Vlad, If you are interested in digging into an issue that is assigned to
me, I'm happy to tell you my status and share what I know about it. I would
certainly welcome your help.
Thanks,
Gail
On Wed, Dec 23, 2015 at 10:51 AM, Gail Badner wrote:
> Hi Vlad,
>
> I've spend quite a bit of time on this one and already have a fix. I just
> have some tests to add to confirm. I will look into what you suggest, but
> please check with me first if you see that an issue is assigned to me.
>
> Thanks,
> Gail
>
> On Wed, Dec 23, 2015 at 4:13 AM, Vlad Mihalcea
> wrote:
>
>> Hi guys
>>
>> I decided to spend some time to investigate the infamous HHH-5855 (
>> https://hibernate.atlassian.net/browse/HHH-5855 ) bug and this is my
>> report.
>>
>> One of the first thing that I noticed is that Sets are fine, while this
>> bug
>> only replicates with bidirectional Bags.
>>
>> After some hours of debugging and logging (since debugging triggers
>> collection initialization), I found the culprit.
>>
>> In the org.hibernate.type.TypeHelper.replace(Object[] original, Object[]
>> target, Type[] types, SessionImplementor session, Object owner, Map
>> copyCache) method, when copying the cached entity state (which contains
>> the
>> newly added child entity along with its identifier) onto the original
>> collection:
>>
>> copied[i] = types[i].replace( original[i], target[i], session, owner,
>> copyCache );
>>
>> inside the org.hibernate.type.CollectionType.replace(Object[] original,
>> Object[] target, Type[] types, SessionImplementor session, Object owner,
>> Map copyCache) method there is this check:
>>
>> if ( !Hibernate.isInitialized( original ) ) {
>>return target;
>> }
>>
>>
>> For Sets, the collection is always initialized because of this line inside
>> the add() method of the org.hibernate.collection.internal.PersistentSet:
>>
>> final Boolean exists = isOperationQueueEnabled() ?
>> readElementExistence( value ) : null;
>>
>> Because of the the readElementExistence( value ) call the Set is always
>> initialized and upon triggering the flush, the newly added Entity being
>> already managed it will be left alone.
>>
>> For PersistentList the aforementioned check is false and the replace never
>> occurs, hence the transient entity lingers in the persistence context and
>> the flush will trigger another insert, so we get duplicates.
>>
>> To make the Bag behave like the Set, all we need to do is to change the
>> add
>> method like this:
>>
>> public boolean add(Object object) {
>>initialize(false);
>>if ( !isOperationQueueEnabled() ) {
>> write();
>> return bag.add( object );
>>}
>>else {
>> queueOperation( new SimpleAdd( object ) );
>> return true;
>>}
>> }
>>
>> but then four tests will fail:
>>
>> org.hibernate.test.legacy.MasterDetailTest > testQueuedBagAdds FAILED
>> java.lang.AssertionError at MasterDetailTest.java:1068
>>
>> org.hibernate.test.unionsubclass.UnionSubclassTest > testUnionSubclass
>> FAILED
>> org.hibernate.ObjectDeletedException at UnionSubclassTest.java:364
>>
>> org.hibernate.test.version.VersionTest > testCollectionNoVersion FAILED
>> java.lang.AssertionError at VersionTest.java:118
>>
>> org.hibernate.test.version.VersionTest > testCollectionVersion FAILED
>> java.lang.AssertionError at VersionTest.java:79
>>
>> 3 of them fail because we expect the List not to be initialized and
>> the UnionSubclassTest fails
>> for a doubtful reason (we attempt to delete an entity which is still
>> referenced).
>>
>> Basically, such a change will finally fix this issue and the Sets and
>> Lists
>> will behave consistently. Since you know the reasons behind the difference
>> in how Sets and Lists are initialized, we need to discuss if this change
>> is
>> appropriate or we should address this issue differently.
>>
>> I have a branch on my fork with a test that replicates this issue and that
>> the proposed change manages to fix it:
>>
>> https://github.com/vladmihalcea/hibernate-orm/tree/feature/hhh5855
>>
>> Let me know what you think and let's discuss it further.
>>
>> Vlad
>> ___
>> 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
Re: [hibernate-dev] HHH-5855 bug report and a possible fix
Hi Gail,
I'm glad there is a development plan on this one . I've been following this
issue for a couple of years and seen some recent comments which reminded me
of it.
Someone asked me on my blog if we can get it fixed, as it's causing
problems when people are trying to merge back detached entities.
If I can help you with anything, just let me know. Now that I've been also
digging into it, I can at least assist you and test your PR on my fork too.
Vlad
On Wed, Dec 23, 2015 at 9:25 PM, Gail Badner wrote:
> We really don't want to initialize a List when merging. Instead, we want
> to do the same sort of replace on the values stored in the DelayedOperation
> objects. That way, the collection will be initialized only when necessary.
> The DelayedOperations are executed on flush. I'll should get a pull request
> out for this today or tomorrow.
>
> Vlad, If you are interested in digging into an issue that is assigned to
> me, I'm happy to tell you my status and share what I know about it. I would
> certainly welcome your help.
>
> Thanks,
> Gail
>
> On Wed, Dec 23, 2015 at 10:51 AM, Gail Badner wrote:
>
>> Hi Vlad,
>>
>> I've spend quite a bit of time on this one and already have a fix. I just
>> have some tests to add to confirm. I will look into what you suggest, but
>> please check with me first if you see that an issue is assigned to me.
>>
>> Thanks,
>> Gail
>>
>> On Wed, Dec 23, 2015 at 4:13 AM, Vlad Mihalcea
>> wrote:
>>
>>> Hi guys
>>>
>>> I decided to spend some time to investigate the infamous HHH-5855 (
>>> https://hibernate.atlassian.net/browse/HHH-5855 ) bug and this is my
>>> report.
>>>
>>> One of the first thing that I noticed is that Sets are fine, while this
>>> bug
>>> only replicates with bidirectional Bags.
>>>
>>> After some hours of debugging and logging (since debugging triggers
>>> collection initialization), I found the culprit.
>>>
>>> In the org.hibernate.type.TypeHelper.replace(Object[] original, Object[]
>>> target, Type[] types, SessionImplementor session, Object owner, Map
>>> copyCache) method, when copying the cached entity state (which contains
>>> the
>>> newly added child entity along with its identifier) onto the original
>>> collection:
>>>
>>> copied[i] = types[i].replace( original[i], target[i], session, owner,
>>> copyCache );
>>>
>>> inside the org.hibernate.type.CollectionType.replace(Object[] original,
>>> Object[] target, Type[] types, SessionImplementor session, Object owner,
>>> Map copyCache) method there is this check:
>>>
>>> if ( !Hibernate.isInitialized( original ) ) {
>>>return target;
>>> }
>>>
>>>
>>> For Sets, the collection is always initialized because of this line
>>> inside
>>> the add() method of the org.hibernate.collection.internal.PersistentSet:
>>>
>>> final Boolean exists = isOperationQueueEnabled() ?
>>> readElementExistence( value ) : null;
>>>
>>> Because of the the readElementExistence( value ) call the Set is always
>>> initialized and upon triggering the flush, the newly added Entity being
>>> already managed it will be left alone.
>>>
>>> For PersistentList the aforementioned check is false and the replace
>>> never
>>> occurs, hence the transient entity lingers in the persistence context and
>>> the flush will trigger another insert, so we get duplicates.
>>>
>>> To make the Bag behave like the Set, all we need to do is to change the
>>> add
>>> method like this:
>>>
>>> public boolean add(Object object) {
>>>initialize(false);
>>>if ( !isOperationQueueEnabled() ) {
>>> write();
>>> return bag.add( object );
>>>}
>>>else {
>>> queueOperation( new SimpleAdd( object ) );
>>> return true;
>>>}
>>> }
>>>
>>> but then four tests will fail:
>>>
>>> org.hibernate.test.legacy.MasterDetailTest > testQueuedBagAdds FAILED
>>> java.lang.AssertionError at MasterDetailTest.java:1068
>>>
>>> org.hibernate.test.unionsubclass.UnionSubclassTest > testUnionSubclass
>>> FAILED
>>> org.hibernate.ObjectDeletedException at UnionSubclassTest.java:364
>>>
>>> org.hibernate.test.version.VersionTest > testCollectionNoVersion FAILED
>>> java.lang.AssertionError at VersionTest.java:118
>>>
>>> org.hibernate.test.version.VersionTest > testCollectionVersion FAILED
>>> java.lang.AssertionError at VersionTest.java:79
>>>
>>> 3 of them fail because we expect the List not to be initialized and
>>> the UnionSubclassTest fails
>>> for a doubtful reason (we attempt to delete an entity which is still
>>> referenced).
>>>
>>> Basically, such a change will finally fix this issue and the Sets and
>>> Lists
>>> will behave consistently. Since you know the reasons behind the
>>> difference
>>> in how Sets and Lists are initialized, we need to discuss if this change
>>> is
>>> appropriate or we should address this issue differently.
>>>
>>> I have a branch on my fork with a test that replicates this issue and
>>> that
>>> the proposed change manages to fix it:
>>>
>>> https://github.com/vladmihalcea/hibernate
[hibernate-dev] Oracle12cDialect identity support
Oracle12cDialect was added in 5.0 by HHH-9044. Unfortunately there are problems with the new identity support that Oracle12cDialect introduced (HHH-9983). The fix for HHH-9983 involved SPI changes, so it was applied to master for 5.1 (only). A couple of weeks ago, Andrea found that identities seemed to work using 5.0 when running BulkManipulationTest using Oracle JDBC 12.1.0.1.0. At the same time it failed for me in the same way as HHH-9983 using 12.1.0.2.0. Then Andrea and I switched the JDBC versions (Andrea tried 12.1.0.1.0; I tried 12.1.0.2.0) and we were both able to run BulkManipulationTest successfully. This was very strange, especially since we were using the same Oracle instance, each using a different user. I didn't see any difference in how we were setting Hibernate properties. Several days later, I tried Oracle JDBC 12.1.0.2.0 again, but BulkManipulationTest was failing again in the same way as HHH-9983. Does anyone know how to get Oracle12cDialect's identity support to work for 5.0 reliably? Is there some version of Oracle JDBC or DB configuration that is required? If so, please let me know. The rest of this email assumes it is not possible. If I'm missing some configuration detail, then just ignore the rest of this message. When the unit tests are run using Oracle12cDialect on 5.0, there are many failures: * tests with entities explicitly mapped to use an identity fail; these failures did not happen in 4.3 using Oracle10gDialect because these tests where skipped (because Oracle10cDialect#supportsIdentityColumns returns false) * use @GeneratedValue with default strategy or use hbm mapping for ID ; these failures did not happen in 4.3 because the native generator for Oracle10gDialect uses a sequence; the native generator for Oracle12cDialect uses an identity. Also, starting in 5.0, when using Oracle 12c, StandardDialectResolver will automatically resolve to Oracle12cDialect if the dialect is not explicitly mapped. Previously it would have resolved ot Oracle10gDialect. I would be OK with changing the native generator in 5.0 from sequence to identity if identities were working properly. I think it will cause confusion when applications break. To get things working with 5.0, applications will need to explicitly set the dialect to Oracle10gDialect, extend Oracle12cDialect to disable identity support, or change ID mappings to specify sequences. Here are some alternatives to get things working. Alternative A: 1) for 5.0 through 5.x override Dialect#getNativeIdentifierGeneratorClass to return SequenceStyleGenerator.class; I suggest the change for 5.x because I don't think it would be an acceptable to change the native generator for Oracle12cDialect from using a sequence to using an identity in 5.x; 2) for 5.0 only, remove support for identities; 3) for 6.0, if desireable, extend Oracle12gDialect (or whatever is current at that time) to change the native generator to use an identity. Alternative B: 1) for 5.0 through 5.x, leave Oracle12cDialect as is, and add a new dialect with native generator that uses a sequence (i.e., Oracle12cNativeSequenceGeneratorDialect or something else that is not so ugly); 2) for 5.0 through 5.x, change StandardDialectResolver to automatically resolve to Oracle12cNativeSequenceGeneratorDialect for Oracle 12g server. Alternative C: 1) for 5.0 through 5.x, change native generator for Oracle12cDialect to use sequence; leave identity support as is (broken in 5.0; fixed in 5.1); 2) for 5.0 through 5.x, add a new dialect with native generator that uses an identity (Oracle12cNativeIdentityGeneratorDialect or something else that is not so ugly). 3) change StandardDialectResolver to automatically resolve to Oracle12cDialect for Oracle 12g server. I prefer alternative A. Alternatives B and C involve maintaining 2 dialects for the same Oracle version; I would think that one of them would be deprecated moving forward. Thoughts? Thanks, Gail ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
