Skip to content

Commit

Permalink
Fix issue with hql and where clause with Embeddable is null
Browse files Browse the repository at this point in the history
  • Loading branch information
dreab8 committed Feb 17, 2020
1 parent efb0750 commit 6cfbed7
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 39 deletions.
Expand Up @@ -33,7 +33,7 @@ public PojoInstantiatorImpl(JavaTypeDescriptor javaTypeDescriptor) {
: resolveConstructor( getMappedPojoClass() );
}

private static Constructor resolveConstructor(Class mappedPojoClass) {
protected static Constructor resolveConstructor(Class mappedPojoClass) {
try {
//noinspection unchecked
return ReflectHelper.getDefaultConstructor( mappedPojoClass);
Expand Down
Expand Up @@ -12,7 +12,9 @@
import org.hibernate.boot.registry.selector.spi.StrategySelector;
import org.hibernate.bytecode.spi.ReflectionOptimizer;
import org.hibernate.cfg.Environment;
import org.hibernate.engine.config.spi.ConfigurationService;
import org.hibernate.internal.util.StringHelper;
import org.hibernate.internal.util.config.ConfigurationHelper;
import org.hibernate.mapping.Backref;
import org.hibernate.mapping.Component;
import org.hibernate.mapping.IndexBackref;
Expand Down Expand Up @@ -47,13 +49,21 @@ public StandardPojoEmbeddableRepresentationStrategy(
creationContext
);


assert bootDescriptor.getComponentClass() != null;

this.strategySelector = creationContext.getSessionFactory()
.getServiceRegistry()
.getService( StrategySelector.class );

this.reflectionOptimizer = buildReflectionOptimizer( bootDescriptor, creationContext );
final ConfigurationService configurationService = creationContext.getMetadata().getMetadataBuildingOptions().getServiceRegistry()
.getService(ConfigurationService.class);
boolean createEmptyCompositesEnabled = ConfigurationHelper.getBoolean(
Environment.CREATE_EMPTY_COMPOSITES_ENABLED,
configurationService.getSettings(),
false
);

if ( reflectionOptimizer != null && reflectionOptimizer.getInstantiationOptimizer() != null ) {
final ReflectionOptimizer.InstantiationOptimizer instantiationOptimizer = reflectionOptimizer.getInstantiationOptimizer();
Expand Down
Expand Up @@ -17,9 +17,12 @@
import java.util.function.Function;

import org.hibernate.NotYetImplementedFor6Exception;
import org.hibernate.cfg.Environment;
import org.hibernate.engine.config.spi.ConfigurationService;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.internal.util.collections.ArrayHelper;
import org.hibernate.internal.util.config.ConfigurationHelper;
import org.hibernate.mapping.Component;
import org.hibernate.mapping.Property;
import org.hibernate.metamodel.mapping.internal.MappingModelCreationHelper;
Expand Down Expand Up @@ -88,6 +91,8 @@ public static EmbeddableMappingType from(

private final EmbeddableValuedModelPart valueMapping;

private final boolean createEmptyCompositesEnabled;

private EmbeddableMappingType(
@SuppressWarnings("unused") Component bootDescriptor,
EmbeddableRepresentationStrategy representationStrategy,
Expand All @@ -99,6 +104,14 @@ private EmbeddableMappingType(

this.valueMapping = embeddedPartBuilder.apply( this );

final ConfigurationService cs = sessionFactory.getServiceRegistry()
.getService(ConfigurationService.class);

this.createEmptyCompositesEnabled = ConfigurationHelper.getBoolean(
Environment.CREATE_EMPTY_COMPOSITES_ENABLED,
cs.getSettings(),
false
);

}

Expand Down Expand Up @@ -392,4 +405,8 @@ public void accept(AttributeMapping attributeMapping) {
}
);
}

public boolean isCreateEmptyCompositesEnabled() {
return createEmptyCompositesEnabled;
}
}
Expand Up @@ -78,6 +78,10 @@ public EmbeddableValuedPathInterpretation(
this.tableGroup = tableGroup;
}

public Expression getSqlExpression() {
return sqlExpression;
}

@Override
public NavigablePath getNavigablePath() {
return sqmPath.getNavigablePath();
Expand Down
Expand Up @@ -20,6 +20,7 @@
import org.hibernate.persister.entity.Loadable;
import org.hibernate.query.QueryLiteralRendering;
import org.hibernate.query.UnaryArithmeticOperator;
import org.hibernate.query.sqm.sql.internal.EmbeddableValuedPathInterpretation;
import org.hibernate.query.sqm.tree.expression.Conversion;
import org.hibernate.sql.ast.Clause;
import org.hibernate.sql.ast.SqlAstWalker;
Expand Down Expand Up @@ -1108,12 +1109,56 @@ public void visitNegatedPredicate(NegatedPredicate negatedPredicate) {

@Override
public void visitNullnessPredicate(NullnessPredicate nullnessPredicate) {
nullnessPredicate.getExpression().accept( this );
if ( nullnessPredicate.isNegated() ) {
appendSql( " is not null" );
final Expression expression = nullnessPredicate.getExpression();
if ( expression instanceof EmbeddableValuedPathInterpretation ) {
final EmbeddableValuedPathInterpretation embeddableValuedPathInterpretation = (EmbeddableValuedPathInterpretation) expression;

final Expression sqlExpression = embeddableValuedPathInterpretation.getSqlExpression();
String predicateValue;
if ( nullnessPredicate.isNegated() ) {
predicateValue = " is not null";
}
else {
predicateValue = " is null";
}
if ( sqlExpression instanceof SqlTuple ) {
SqlTuple tuple = (SqlTuple) sqlExpression;
String separator = NO_SEPARATOR;

boolean isCurrentWhereClause = clauseStack.getCurrent() == Clause.WHERE;
if ( isCurrentWhereClause ) {
appendSql( OPEN_PARENTHESIS );
}

for ( Expression exp : tuple.getExpressions() ) {
appendSql( separator );
exp.accept( this );
appendSql( predicateValue );
separator = " and ";
}

if ( isCurrentWhereClause ) {
appendSql( CLOSE_PARENTHESIS );
}
}
else {
expression.accept( this );
if ( nullnessPredicate.isNegated() ) {
appendSql( " is not null" );
}
else {
appendSql( " is null" );
}
}
}
else {
appendSql( " is null" );
expression.accept( this );
if ( nullnessPredicate.isNegated() ) {
appendSql( " is not null" );
}
else {
appendSql( " is null" );
}
}
}

Expand Down
Expand Up @@ -10,6 +10,9 @@
import java.util.Map;
import java.util.function.Consumer;

import org.hibernate.cfg.Environment;
import org.hibernate.engine.config.spi.ConfigurationService;
import org.hibernate.internal.util.config.ConfigurationHelper;
import org.hibernate.metamodel.mapping.EmbeddableMappingType;
import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart;
import org.hibernate.metamodel.mapping.SingularAttributeMapping;
Expand All @@ -36,6 +39,7 @@ public abstract class AbstractEmbeddableInitializer extends AbstractFetchParentA

// per-row state
private final Object[] resolvedValues;
private final boolean createEmptyCompositesEnabled;
private Object compositeInstance;


Expand Down Expand Up @@ -66,6 +70,8 @@ public AbstractEmbeddableInitializer(
}
);

createEmptyCompositesEnabled = embeddableTypeDescriptor.isCreateEmptyCompositesEnabled();

}

@Override
Expand Down Expand Up @@ -136,20 +142,29 @@ public void initializeInstance(RowProcessingState rowProcessingState) {
compositeInstance
);

boolean areAllValuesNull = true;
for ( Map.Entry<StateArrayContributorMapping, DomainResultAssembler> entry : assemblerMap.entrySet() ) {
final Object contributorValue = entry.getValue().assemble(
rowProcessingState,
rowProcessingState.getJdbcValuesSourceProcessingState().getProcessingOptions()
);

resolvedValues[ entry.getKey().getStateArrayPosition() ] = contributorValue;
if ( contributorValue != null ) {
areAllValuesNull = false;
}
}

embeddedModelPartDescriptor.getEmbeddableTypeDescriptor().setPropertyValues(
compositeInstance,
resolvedValues
);

if ( !createEmptyCompositesEnabled && areAllValuesNull ) {
compositeInstance = null;
}
else {
embeddedModelPartDescriptor.getEmbeddableTypeDescriptor().setPropertyValues(
compositeInstance,
resolvedValues
);
}
}

@Override
Expand Down
Expand Up @@ -359,9 +359,11 @@ public void testDottedProperty(SessionFactoryScope scope) {
// Create short swap
Swap shortSwap = new Swap();
shortSwap.setTenor( 2 );

FixedLeg shortFixed = new FixedLeg();
shortFixed.setPaymentFrequency( Frequency.SEMIANNUALLY );
shortFixed.setRate( 5.6 );

FloatLeg shortFloating = new FloatLeg();
shortFloating.setPaymentFrequency( Frequency.QUARTERLY );
shortFloating.setRateIndex( RateIndex.LIBOR );
Expand All @@ -371,9 +373,11 @@ public void testDottedProperty(SessionFactoryScope scope) {
// Create medium swap
Swap swap = new Swap();
swap.setTenor( 7 );

FixedLeg fixed = new FixedLeg();
fixed.setPaymentFrequency( Frequency.MONTHLY );
fixed.setRate( 7.6 );

FloatLeg floating = new FloatLeg();
floating.setPaymentFrequency( Frequency.MONTHLY );
floating.setRateIndex( RateIndex.TIBOR );
Expand All @@ -383,9 +387,11 @@ public void testDottedProperty(SessionFactoryScope scope) {
// Create long swap
Swap longSwap = new Swap();
longSwap.setTenor( 7 );

FixedLeg longFixed = new FixedLeg();
longFixed.setPaymentFrequency( Frequency.MONTHLY );
longFixed.setRate( 7.6 );

FloatLeg longFloating = new FloatLeg();
longFloating.setPaymentFrequency( Frequency.MONTHLY );
longFloating.setRateIndex( RateIndex.TIBOR );
Expand Down Expand Up @@ -480,35 +486,25 @@ public void testEmbeddedInSecondaryTable(SessionFactoryScope scope) {

@Test
public void testParent(SessionFactoryScope scope) {
scope.inSession(
Book book = new Book();
scope.inTransaction(
session -> {
session.getTransaction().begin();
try {
Book book = new Book();
book.setIsbn( "1234" );
book.setName( "HiA Second Edition" );
Summary summary = new Summary();
summary.setText( "This is a HiA SE summary" );
summary.setSize( summary.getText().length() );
book.setSummary( summary );
session.persist( book );
session.getTransaction().commit();

session.clear();
book.setIsbn( "1234" );
book.setName( "HiA Second Edition" );
Summary summary = new Summary();
summary.setText( "This is a HiA SE summary" );
summary.setSize( summary.getText().length() );
book.setSummary( summary );
session.persist( book );
}
);

Transaction tx = session.beginTransaction();
Book loadedBook = session.get( Book.class, book.getIsbn() );
assertNotNull( loadedBook.getSummary() );
assertEquals( loadedBook, loadedBook.getSummary().getSummarizedBook() );
session.delete( loadedBook );
tx.commit();
}
catch (Exception e) {
if ( session.getTransaction().isActive() ) {
session.getTransaction().rollback();
}
throw e;
}
scope.inTransaction(
session -> {
Book loadedBook = session.get( Book.class, book.getIsbn() );
assertNotNull( loadedBook.getSummary() );
assertEquals( loadedBook, loadedBook.getSummary().getSummarizedBook() );
session.delete( loadedBook );
}
);
}
Expand Down
Expand Up @@ -69,7 +69,7 @@ public void testQuery(SessionFactoryScope scope) {
ForeignCustomer.class
).getSingleResult();
assertThat( foreignCustomer.getName(), is( "foreign" ) );
assertThat( foreignCustomer.getAddress().getCity(), is( nullValue() ) );
assertThat( foreignCustomer.getAddress(), is( nullValue() ) );
}
);
}
Expand Down
Expand Up @@ -75,7 +75,7 @@ public void testQuery(SessionFactoryScope scope) {
ForeignCustomer.class
).getSingleResult();
assertThat( foreignCustomer.getName(), is( "foreign" ) );
assertThat( foreignCustomer.getAddress().getCity(), is( nullValue() ) );
assertThat( foreignCustomer.getAddress(), is( nullValue() ) );
}
);

Expand Down
Expand Up @@ -85,7 +85,7 @@ public void testSubselect(SessionFactoryScope scope) {

// None of the collections should be loaded yet
for ( Parent p : list ) {
assertFalse( Hibernate.isInitialized( list.get( 0 ).children ) );
assertFalse( Hibernate.isInitialized( p.children ) );
}

// When the first collection is loaded, the full batch of 50 collections
Expand Down

0 comments on commit 6cfbed7

Please sign in to comment.