ElementCollection is missing elements after update if embeddable type has primitive columns marked as nullable

Description

Test case below.

We had an Embeddable class containing primitive boolean properties (which may be uncommon in Java but common in Kotlin as non-nullable Kotlin Boolean maps to primitive Java boolean. Still, this issue is not Kotlin-specific and the test case below is written in Java.) We forgot to mark the @Column annotation as nullable = false. This leads to unexpected behavior: When updating a single element from the collection, all unchanged elements are deleted and only the changed element remains in the collection after flushing.

It seems that Hibernate considers the boolean columns nullable when building the delete statement so it does not include them, but considers them not nullable when determining whether it has to insert all remaining elements of the collection after a change.

As a side note, I think it is somewhat dangerous to assume there is a unique key consisting of all non-nullable columns of the embeddable plus the foreign key. It would be safer to use the delete-all-insert-all behavior by default and have a Hibernate-specific annotation to explicitly define a unique key if one is present.

Test case:

/*
 * Copyright 2014 JBoss Inc
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.hibernate.bugs;

import jakarta.persistence.Column;
import jakarta.persistence.ElementCollection;
import jakarta.persistence.Embeddable;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.Id;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.cfg.Configuration;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
import org.junit.Test;

import java.util.Objects;
import java.util.Set;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThat;

/**
 * This template demonstrates how to develop a test case for Hibernate ORM, using its built-in unit test framework.
 * Although ORMStandaloneTestCase is perfectly acceptable as a reproducer, usage of this class is much preferred.
 * Since we nearly always include a regression test with bug fixes, providing your reproducer using this method
 * simplifies the process.
 *
 * What's even better?  Fork hibernate-orm itself, add your test case directly to a module's unit tests, then
 * submit it as a PR!
 */
public class ORMUnitTestCase extends BaseCoreFunctionalTestCase {

    @Entity
    public static class MyEntity {
        @Id
        private String id;

        @ElementCollection(fetch = FetchType.EAGER)
        private Set<MyEmbeddable> elementCollection;

        public MyEntity() {
        }

        public MyEntity(String id, Set<MyEmbeddable> elementCollection) {
            this.id = id;
            this.elementCollection = elementCollection;
        }

        public String getId() {
            return id;
        }

        public void setId(String id) {
            this.id = id;
        }

        public Set<MyEmbeddable> getElementCollection() {
            return elementCollection;
        }

        public void setElementCollection(Set<MyEmbeddable> elementCollection) {
            this.elementCollection = elementCollection;
        }
    }

    @Embeddable
    public static class MyEmbeddable {
        @Column
        private String stringField;

        @Column
        private boolean booleanField;

        public MyEmbeddable() {
        }

        public MyEmbeddable(String stringField, boolean booleanField) {
            this.stringField = stringField;
            this.booleanField = booleanField;
        }

        public String getStringField() {
            return stringField;
        }

        public void setStringField(String stringField) {
            this.stringField = stringField;
        }

        public boolean isBooleanField() {
            return booleanField;
        }

        public void setBooleanField(boolean booleanField) {
            this.booleanField = booleanField;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) {
                return true;
            }
            if (o == null || getClass() != o.getClass()) {
                return false;
            }
            MyEmbeddable that = (MyEmbeddable) o;
            return booleanField == that.booleanField && Objects.equals(stringField, that.stringField);
        }

        @Override
        public int hashCode() {
            return Objects.hash(stringField, booleanField);
        }
    }

    // Add your entities here.
    @Override
    protected Class<?>[] getAnnotatedClasses() {
        return new Class[] {
            MyEntity.class,
            MyEmbeddable.class
        };
    }

    // Add in any settings that are specific to your test.  See resources/hibernate.properties for the defaults.
    @Override
    protected void configure(Configuration configuration) {
        super.configure(configuration);

        configuration.setProperty(AvailableSettings.SHOW_SQL, Boolean.TRUE.toString());
        configuration.setProperty(AvailableSettings.FORMAT_SQL, Boolean.TRUE.toString());
        //configuration.setProperty( AvailableSettings.GENERATE_STATISTICS, "true" );
    }

    // Add your tests, using standard JUnit.
    @Test
    public void hhh123Test() throws Exception {
        String id = UUID.randomUUID().toString();
        inTransaction(session -> {
            MyEntity myEntity = new MyEntity(id, Set.of(
                new MyEmbeddable("1", false),
                new MyEmbeddable("2", false)
            ));
            session.persist(myEntity);
        });

        inTransaction(session -> {
            MyEntity myEntity = session.find(MyEntity.class, id);
            assertThat(myEntity.getElementCollection()).hasSize(2);

            myEntity.getElementCollection().stream().filter(it -> it.getStringField().equals("1")).forEach(it -> it.setBooleanField(true));
        });

        inTransaction(session -> {
            MyEntity myEntity = session.find(MyEntity.class, id);
            assertThat(myEntity.getElementCollection()).hasSize(2);
            assertThat(myEntity.getElementCollection()).extracting(MyEmbeddable::isBooleanField).containsExactlyInAnyOrder(true, false);
        });
    }
}

Activity

Show:

Lars Bilger October 2, 2023 at 8:47 PM

@Gavin King I agree that there should be no nullable key fields, but what’s a key field in this context? Hibernate seems to assume that the key of an ElementCollection is composed of all non-nullable fields.

Maybe there should be a startup error if there are primitive fields marked as nullable. Or the nullability from the @Column annotation should be silently overridden if the type clearly indicates a non-null field (like primitive types or even non-null Kotlin types). I think that would solve this problem, as it only occurs if a primitive field is marked nullable by the @Column annotation.

Gavin King September 29, 2023 at 12:54 PM

@Lars Bilger @Andrea Boriero I think it’s conceptually wrong to have a nullable key field, and this should produce an error at startup time.

Andrea Boriero September 28, 2023 at 11:13 AM

Hi @Lars Bilger ,

thanks for checking and spotting the issue, I’m working on it!

Lars Bilger September 27, 2023 at 8:52 PM

@Andrea Boriero thanks for working on this so quickly! I saw your PR and I think it still breaks if the “key” field is a primitive and marked as nullable in the @Column annotation. If I change one of your tests as follows, it fails (I only changed the stringField to be an int):

package org.hibernate.orm.test.embeddable;

import java.util.Objects;
import java.util.Set;

import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.JiraKey;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import jakarta.persistence.Column;
import jakarta.persistence.ElementCollection;
import jakarta.persistence.Embeddable;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.Id;

import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;

@DomainModel(
		annotatedClasses = {
				ElementCollectionNonOptionalPropertyUpdateTest.MyEntity.class
		}
)
@SessionFactory
@JiraKey( "HHH-17257" )
public class ElementCollectionNonOptionalPropertyUpdateTest {

	private static final Long ENTITY_ID = 1l;

	@BeforeAll
	public void setUp(SessionFactoryScope scope) {
		scope.inTransaction(
				session -> {
					MyEntity myEntity = new MyEntity( ENTITY_ID, Set.of(
							new MyEmbeddable( 1, false ),
							new MyEmbeddable( 2, false )
					) );
					session.persist( myEntity );

				}
		);
	}

	@Test
	public void testUpdateElement(SessionFactoryScope scope) {

		scope.inTransaction(
				session -> {
					MyEntity myEntity = session.find( MyEntity.class, ENTITY_ID );
					Set<MyEmbeddable> elementCollection = myEntity.getElementCollection();
					assertThat( elementCollection ).hasSize( 2 );

					elementCollection.stream().filter( it -> it.getIntField() == 1 ).forEach(
							it -> it.setBooleanField( true ) );
				}
		);

		scope.inTransaction(
				session -> {
					MyEntity myEntity = session.find( MyEntity.class, ENTITY_ID );
					assertThat( myEntity.getElementCollection() ).hasSize( 2 );
					assertThat( myEntity.getElementCollection() ).extracting( MyEmbeddable::isBooleanField )
							.containsExactlyInAnyOrder( true, false );
				}
		);
	}


	@Entity(name = "MyEntity")
	public static class MyEntity {
		@Id
		private Long id;

		@ElementCollection(fetch = FetchType.EAGER)
		private Set<MyEmbeddable> elementCollection;

		public MyEntity() {
		}

		public MyEntity(Long id, Set<MyEmbeddable> elementCollection) {
			this.id = id;
			this.elementCollection = elementCollection;
		}

		public Long getId() {
			return id;
		}

		public Set<MyEmbeddable> getElementCollection() {
			return elementCollection;
		}

		public void setElementCollection(Set<MyEmbeddable> elementCollection) {
			this.elementCollection = elementCollection;
		}
	}

	@Embeddable
	public static class MyEmbeddable {
		@Column
		private int intField;

		@Column
		// the field is considered non-optional because of its primitive type
		private boolean booleanField;

		public MyEmbeddable() {
		}

		public MyEmbeddable(int intField, boolean booleanField) {
			this.intField = intField;
			this.booleanField = booleanField;
		}

		public int getIntField() {
			return intField;
		}

		public void setIntField(int stringField) {
			this.intField = stringField;
		}

		public boolean isBooleanField() {
			return booleanField;
		}

		public void setBooleanField(boolean booleanField) {
			this.booleanField = booleanField;
		}

		@Override
		public boolean equals(Object o) {
			if ( this == o ) {
				return true;
			}
			if ( o == null || getClass() != o.getClass() ) {
				return false;
			}
			MyEmbeddable that = (MyEmbeddable) o;
			return booleanField == that.booleanField && Objects.equals(intField, that.intField);
		}

		@Override
		public int hashCode() {
			return Objects.hash(intField, booleanField );
		}
	}

}

Fixed

Details
Assignee
Andrea Boriero
Reporter
Lars Bilger
Sprint
None
Fix versions
6.3.2
6.2.11
Affects versions
6.3.1
Priority
Major

Created September 25, 2023 at 10:54 AM

Updated October 5, 2023 at 12:28 AM

Resolved September 27, 2023 at 3:29 PM

ElementCollection is missing elements after update if embeddable type has primitive columns marked as nullable

Description

Activity

Lars Bilger October 2, 2023 at 8:47 PM

Gavin King September 29, 2023 at 12:54 PM

Andrea Boriero September 28, 2023 at 11:13 AM

Lars Bilger September 27, 2023 at 8:52 PM

Details
Assignee
Andrea Boriero
Reporter
Lars Bilger
Sprint
None
Fix versions
6.3.2
6.2.11
Affects versions
6.3.1
Priority
Major

Details

Assignee

Reporter

Sprint

Fix versions

Affects versions

Priority

Flag notifications

Something's gone wrong

ElementCollection is missing elements after update if embeddable type has primitive columns marked as nullable

Description

Activity

Lars Bilger October 2, 2023 at 8:47 PM

Gavin King September 29, 2023 at 12:54 PM

Andrea Boriero September 28, 2023 at 11:13 AM

Lars Bilger September 27, 2023 at 8:52 PM

DetailsAssigneeAndrea BorieroAndrea BorieroReporterLars BilgerLars BilgerSprintNone+1Fix versions6.3.26.2.11Affects versions6.3.1PriorityMajor

Details

Assignee

Reporter

Sprint

Fix versions

Affects versions

Priority

Flag notifications

Something's gone wrong

Details
Assignee
Andrea Boriero
Reporter
Lars Bilger
Sprint
None
Fix versions
6.3.2
6.2.11
Affects versions
6.3.1
Priority
Major