From cf1bf5f8235cafbedb7e9b2f254d223f18a8b0c7 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Tue, 8 Mar 2022 18:11:49 -0600 Subject: [PATCH] HHH-15106 - fk() HQL function --- hibernate-core/src/main/antlr/hql-sql.g | 23 +- hibernate-core/src/main/antlr/hql.g | 10 +- hibernate-core/src/main/antlr/sql-gen.g | 1 + .../hql/internal/ast/SqlASTFactory.java | 4 + .../hql/internal/ast/tree/DotNode.java | 10 +- .../hql/internal/ast/tree/FkRefNode.java | 133 +++++++++ .../orm/test/notfound/FkRefTests.java | 271 ++++++++++++++++++ 7 files changed, 444 insertions(+), 8 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FkRefNode.java create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/notfound/FkRefTests.java diff --git a/hibernate-core/src/main/antlr/hql-sql.g b/hibernate-core/src/main/antlr/hql-sql.g index 5265979d5e..c079184dfb 100644 --- a/hibernate-core/src/main/antlr/hql-sql.g +++ b/hibernate-core/src/main/antlr/hql-sql.g @@ -244,6 +244,15 @@ tokens return dot; } + protected AST lookupFkRefSource(AST path) throws SemanticException { + if ( path.getType() == DOT ) { + return lookupProperty( path, true, isInSelect() ); + } + else { + return lookupNonQualifiedProperty( path ); + } + } + protected boolean isNonQualifiedPropertyRef(AST ident) { return false; } protected AST lookupNonQualifiedProperty(AST property) throws SemanticException { return property; } @@ -713,12 +722,15 @@ identifier ; addrExpr! [ boolean root ] - : #(d:DOT lhs:addrExprLhs rhs:propertyName ) { + : #(d:DOT lhs:addrExprLhs rhs:propertyName ) { // This gives lookupProperty() a chance to transform the tree // to process collection properties (.elements, etc). #addrExpr = #(#d, #lhs, #rhs); #addrExpr = lookupProperty(#addrExpr,root,false); } + | fk_ref:fkRef { + #addrExpr = #fk_ref; + } | #(i:INDEX_OP lhs2:addrExprLhs rhs2:expr [ null ]) { #addrExpr = #(#i, #lhs2, #rhs2); processIndex(#addrExpr); @@ -743,6 +755,12 @@ addrExpr! [ boolean root ] } ; +fkRef + : #( r:FK_REF p:propertyRef ) { + #p = lookupProperty( #p, false, isInSelect() ); + } + ; + addrExprLhs : addrExpr [ false ] ; @@ -764,8 +782,7 @@ propertyRef! #propertyRef = #(#d, #lhs, #rhs); #propertyRef = lookupProperty(#propertyRef,false,true); } - | - p:identifier { + | p:identifier { // In many cases, things other than property-refs are recognized // by this propertyRef rule. Some of those I have seen: // 1) select-clause from-aliases diff --git a/hibernate-core/src/main/antlr/hql.g b/hibernate-core/src/main/antlr/hql.g index ac9e2a4ac3..136b0ab816 100644 --- a/hibernate-core/src/main/antlr/hql.g +++ b/hibernate-core/src/main/antlr/hql.g @@ -46,6 +46,7 @@ tokens EXISTS="exists"; FALSE="false"; FETCH="fetch"; + FK_REF; FROM="from"; FULL="full"; GROUP="group"; @@ -721,7 +722,8 @@ atom // level 0 - the basic element of an expression primaryExpression - : { validateSoftKeyword("function") && LA(2) == OPEN && LA(3) == QUOTED_STRING }? jpaFunctionSyntax + : { validateSoftKeyword("fk") && LA(2) == OPEN }? fkRefPath + | { validateSoftKeyword("function") && LA(2) == OPEN && LA(3) == QUOTED_STRING }? jpaFunctionSyntax | { validateSoftKeyword("cast") && LA(2) == OPEN }? castFunction | identPrimary ( options {greedy=true;} : DOT^ "class" )? | constant @@ -729,6 +731,12 @@ primaryExpression | OPEN! (expressionOrVector | subQuery) CLOSE! ; +fkRefPath! + : "fk" OPEN p:identPrimary CLOSE { + #fkRefPath = #( [FK_REF], #p ); + } + ; + jpaFunctionSyntax! : i:IDENT OPEN n:QUOTED_STRING (COMMA a:exprList)? CLOSE { final String functionName = unquote( #n.getText() ); diff --git a/hibernate-core/src/main/antlr/sql-gen.g b/hibernate-core/src/main/antlr/sql-gen.g index 40d48f0343..1a409c41e3 100644 --- a/hibernate-core/src/main/antlr/sql-gen.g +++ b/hibernate-core/src/main/antlr/sql-gen.g @@ -500,6 +500,7 @@ parameter addrExpr : #(r:DOT . .) { out(r); } + | #(fk:FK_REF .) { out(fk); } | i:ALIAS_REF { out(i); } | j:INDEX_OP { out(j); } | v:RESULT_VARIABLE_REF { out(v); } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/SqlASTFactory.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/SqlASTFactory.java index acf4146b1f..7b038ccc7b 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/SqlASTFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/SqlASTFactory.java @@ -15,6 +15,7 @@ import org.hibernate.hql.internal.ast.tree.BinaryArithmeticOperatorNode; import org.hibernate.hql.internal.ast.tree.BinaryLogicOperatorNode; import org.hibernate.hql.internal.ast.tree.BooleanLiteralNode; import org.hibernate.hql.internal.ast.tree.CastFunctionNode; +import org.hibernate.hql.internal.ast.tree.FkRefNode; import org.hibernate.hql.internal.ast.tree.NullNode; import org.hibernate.hql.internal.ast.tree.SearchedCaseNode; import org.hibernate.hql.internal.ast.tree.SimpleCaseNode; @@ -196,6 +197,9 @@ public class SqlASTFactory extends ASTFactory implements HqlSqlTokenTypes { case NULL : { return NullNode.class; } + case FK_REF: { + return FkRefNode.class; + } default: return SqlNode.class; } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java index 9669ee0ea8..65b7c64b39 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java @@ -9,6 +9,7 @@ package org.hibernate.hql.internal.ast.tree; import org.hibernate.QueryException; import org.hibernate.engine.internal.JoinSequence; import org.hibernate.hql.internal.CollectionProperties; +import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes; import org.hibernate.hql.internal.antlr.SqlTokenTypes; import org.hibernate.hql.internal.ast.util.ASTUtil; import org.hibernate.hql.internal.ast.util.ColumnHelper; @@ -260,7 +261,7 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec } private Type prepareLhs() throws SemanticException { - FromReferenceNode lhs = getLhs(); + final FromReferenceNode lhs = getLhs(); lhs.prepareForDot( propertyName ); return getDataType(); } @@ -390,10 +391,11 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec final boolean joinIsNeeded; if ( isDotNode( parent ) ) { - // our parent is another dot node, meaning we are being further dereferenced. - // thus we need to generate a join unless the association is non-nullable and - // parent refers to the associated entity's PK (because 'our' table would know the FK). parentAsDotNode = (DotNode) parent; + + // our parent is another dot node, meaning we are being further de-referenced. + // depending on the exact de-reference we may need to generate a physical join. + property = parentAsDotNode.propertyName; joinIsNeeded = generateJoin && ( entityType.isNullable() || diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FkRefNode.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FkRefNode.java new file mode 100644 index 0000000000..9e5e2c4d91 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FkRefNode.java @@ -0,0 +1,133 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.hql.internal.ast.tree; + +import org.hibernate.QueryException; +import org.hibernate.hql.internal.ast.InvalidPathException; +import org.hibernate.type.BasicType; +import org.hibernate.type.CompositeType; +import org.hibernate.type.ManyToOneType; +import org.hibernate.type.Type; + +import antlr.SemanticException; +import antlr.collections.AST; + +/** + * Represents a `fk()` pseudo-function + * + * @author Steve Ebersole + */ +public class FkRefNode + extends HqlSqlWalkerNode + implements ResolvableNode, DisplayableNode, PathNode { + private FromReferenceNode toOnePath; + + private Type fkType; + private String[] columns; + + private FromReferenceNode resolveToOnePath() { + if ( toOnePath == null ) { + try { + resolve( false, true ); + } + catch (SemanticException e) { + final String msg = "Unable to resolve to-one path `fk(" + toOnePath.getPath() + "`)"; + throw new QueryException( msg, new InvalidPathException( msg ) ); + } + } + + assert toOnePath != null; + return toOnePath; + } + + @Override + public String getDisplayText() { + final FromReferenceNode toOnePath = resolveToOnePath(); + return "fk(`" + toOnePath.getDisplayText() + "` )"; + } + + @Override + public String getPath() { + return toOnePath.getDisplayText() + ".{fk}"; + } + + @Override + public void resolve( + boolean generateJoin, + boolean implicitJoin) throws SemanticException { + if ( toOnePath != null ) { + return; + } + + final AST firstChild = getFirstChild(); + assert firstChild instanceof FromReferenceNode; + + toOnePath = (FromReferenceNode) firstChild; + toOnePath.resolve( false, true, null, toOnePath.getFromElement() ); + + final Type sourcePathDataType = toOnePath.getDataType(); + if ( ! ( sourcePathDataType instanceof ManyToOneType ) ) { + throw new InvalidPathException( + "Argument to fk() function must be a to-one path, but found " + sourcePathDataType + ); + } + final ManyToOneType toOneType = (ManyToOneType) sourcePathDataType; + final FromElement fromElement = toOnePath.getFromElement(); + + fkType = toOneType.getIdentifierOrUniqueKeyType( getSessionFactoryHelper().getFactory() ); + assert fkType instanceof BasicType + || fkType instanceof CompositeType; + + columns = fromElement.toColumns( + fromElement.getTableAlias(), + toOneType.getPropertyName(), + getWalker().isInSelect() + ); + assert columns != null && columns.length > 0; + + setText( String.join( ", ", columns ) ); + } + + @Override + public void resolve( + boolean generateJoin, + boolean implicitJoin, + String classAlias, + AST parent, + AST parentPredicate) throws SemanticException { + resolve( false, true ); + } + + @Override + public void resolve( + boolean generateJoin, + boolean implicitJoin, + String classAlias, + AST parent) throws SemanticException { + resolve( false, true ); + } + + @Override + public void resolve( + boolean generateJoin, + boolean implicitJoin, + String classAlias) throws SemanticException { + resolve( false, true ); + } + + @Override + public void resolveInFunctionCall( + boolean generateJoin, + boolean implicitJoin) throws SemanticException { + resolve( false, true ); + } + + @Override + public void resolveIndex(AST parent) throws SemanticException { + throw new InvalidPathException( "fk() paths cannot be de-referenced as indexed path" ); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/notfound/FkRefTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/notfound/FkRefTests.java new file mode 100644 index 0000000000..44458e4872 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/notfound/FkRefTests.java @@ -0,0 +1,271 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.orm.test.notfound; + +import java.io.Serializable; +import java.util.List; +import javax.persistence.ConstraintMode; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.ForeignKey; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.OneToOne; + +import org.hibernate.QueryException; +import org.hibernate.annotations.NotFound; +import org.hibernate.annotations.NotFoundAction; +import org.hibernate.boot.SessionFactoryBuilder; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.jdbc.SQLStatementInterceptor; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.fail; + +/** + * Tests for the new `{fk}` HQL token + * + * @author Steve Ebersole + */ +public class FkRefTests extends BaseNonConfigCoreFunctionalTestCase { + + private SQLStatementInterceptor statementInspector; + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { + Coin.class, Currency.class + }; + } + + @Override + protected void configureSessionFactoryBuilder(SessionFactoryBuilder sfb) { + statementInspector = new SQLStatementInterceptor( sfb ); + } + + + @Test + @TestForIssue(jiraKey = "HHH-15106") + public void testSimplePredicateUse() { + statementInspector.clear(); + + // there is a Coin which has a currency_fk = 1 + inTransaction( (session) -> { + final String hql = "select c from Coin c where fk(c.currency) = 1"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + assertThat( coins.size(), is( 1 ) ); + assertThat( coins.get( 0 ), notNullValue() ); + assertThat( coins.get( 0 ).getCurrency(), nullValue() ); + + assertThat( statementInspector.getSqlQueries().size(), is( 2 ) ); + assertThat( statementInspector.getSqlQueries().get( 0 ), not( containsString( " join " ) ) ); + } ); + + statementInspector.clear(); + + // However, the "matching" Currency does not exist + inTransaction( (session) -> { + final String hql = "select c from Coin c where c.currency.id = 1"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + assertThat( coins.size(), is( 0 ) ); + } ); + + statementInspector.clear(); + + // check using `currency` as a naked "property-ref" + inTransaction( (session) -> { + final String hql = "select c from Coin c where fk(currency) = 1"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + assertThat( coins.size(), is( 1 ) ); + assertThat( coins.get( 0 ), notNullValue() ); + assertThat( coins.get( 0 ).getCurrency(), nullValue() ); + + assertThat( statementInspector.getSqlQueries().size(), is( 2 ) ); + assertThat( statementInspector.getSqlQueries().get( 0 ), not( containsString( " join " ) ) ); + } ); + } + + @Test + @TestForIssue(jiraKey = "HHH-15106") + public void testNullnessPredicateUse() { + statementInspector.clear(); + + // there is one Coin (id=3) which has a null currency_fk + inTransaction( (session) -> { + final String hql = "select c from Coin c where fk(c.currency) is null"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + assertThat( coins.size(), is( 1 ) ); + assertThat( coins.get( 0 ), notNullValue() ); + assertThat( coins.get( 0 ).getId(), is( 3 ) ); + assertThat( coins.get( 0 ).getCurrency(), nullValue() ); + + assertThat( statementInspector.getSqlQueries().size(), is( 1 ) ); + assertThat( statementInspector.getSqlQueries().get( 0 ), not( containsString( " join " ) )); + } ); + + statementInspector.clear(); + + // check using `currency` as a naked "property-ref" + inTransaction( (session) -> { + final String hql = "select c from Coin c where fk(currency) is null"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + assertThat( coins.size(), is( 1 ) ); + assertThat( coins.get( 0 ), notNullValue() ); + assertThat( coins.get( 0 ).getId(), is( 3 ) ); + assertThat( coins.get( 0 ).getCurrency(), nullValue() ); + + assertThat( statementInspector.getSqlQueries().size(), is( 1 ) ); + assertThat( statementInspector.getSqlQueries().get( 0 ), not( containsString( " join " ) )); + } ); + } + + @Test + @TestForIssue(jiraKey = "HHH-15106") + public void testFkRefDereferenceNotAllowed() { + statementInspector.clear(); + + inTransaction( (session) -> { + try { + final String hql = "select c from Coin c where fk(c.currency).something"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + fail( "Expecting failure" ); + } + catch (IllegalArgumentException expected) { + assertThat( expected.getCause(), instanceOf( QueryException.class ) ); + } + catch (Exception e) { + fail( "Unexpected failure type : " + e ); + } + } ); + + inTransaction( (session) -> { + try { + final String hql = "select c from Coin c where currency.{fk}.something"; + final List coins = session.createQuery( hql, Coin.class ).getResultList(); + } + catch (IllegalArgumentException expected) { + assertThat( expected.getCause(), instanceOf( QueryException.class ) ); + } + } ); + } + + @Before + public void prepareTestData() { + inTransaction( (session) -> { + Currency euro = new Currency( 1, "Euro" ); + Coin fiveC = new Coin( 1, "Five cents", euro ); + session.persist( euro ); + session.persist( fiveC ); + + Currency usd = new Currency( 2, "USD" ); + Coin penny = new Coin( 2, "Penny", usd ); + session.persist( usd ); + session.persist( penny ); + + Coin noCurrency = new Coin( 3, "N/A", null ); + session.persist( noCurrency ); + } ); + + inTransaction( (session) -> { + session.createQuery( "delete Currency where id = 1" ).executeUpdate(); + } ); + } + + @After + public void cleanupTest() throws Exception { + inTransaction( (session) -> { + session.createQuery( "delete Coin" ).executeUpdate(); + session.createQuery( "delete Currency" ).executeUpdate(); + } ); + } + + @Entity(name = "Coin") + public static class Coin { + private Integer id; + private String name; + private Currency currency; + + public Coin() { + } + + public Coin(Integer id, String name, Currency currency) { + this.id = id; + this.name = name; + this.currency = currency; + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + @OneToOne(fetch = FetchType.LAZY) + @NotFound(action = NotFoundAction.IGNORE) + @JoinColumn(name = "currency_fk", foreignKey = @ForeignKey(value = ConstraintMode.NO_CONSTRAINT)) + public Currency getCurrency() { + return currency; + } + + public void setCurrency(Currency currency) { + this.currency = currency; + } + } + + @Entity(name = "Currency") + public static class Currency implements Serializable { + private Integer id; + private String name; + + public Currency() { + } + + public Currency(Integer id, String name) { + this.id = id; + this.name = name; + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +}