From d4b4667271fbedefb94c82d25dd1e06597ad1c53 Mon Sep 17 00:00:00 2001 From: Ajay Paul Date: Tue, 29 Oct 2024 16:25:35 +0530 Subject: [PATCH 1/4] Fix for issue 2211 --- .../jpql/JPQLFunctionsAbstractBuilder.java | 86 ++++++++++++++----- .../internal/jpa/jpql/ReportItemBuilder.java | 3 +- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java index 90e3c118458..ac0dbb9f067 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java @@ -28,10 +28,14 @@ import java.util.List; /** - * JPQL exclusive ID(), VERSION() functions/expressions are transformed there to StateFieldPathExpression. - * It should be used in the future for another JPQL functions/expressions which are not available at the DB level. - * E.g. For Entity e with idAttr as a primary key: SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e - * For Entity e with versionAttr as a version attribute: SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e + * JPQL exclusive ID(), VERSION() functions/expressions are transformed there to + * StateFieldPathExpression. + * It should be used in the future for another JPQL functions/expressions which + * are not available at the DB level. + * E.g. For Entity e with idAttr as a primary key: + * SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e + * For Entity e with versionAttr as a version attribute: + * SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e * * @author Radek Felcman * @since 5.0 @@ -39,7 +43,8 @@ public abstract class JPQLFunctionsAbstractBuilder extends EclipseLinkAnonymousExpressionVisitor { /** - * The {@link JPQLQueryContext} is used to query information about the application metadata and + * The {@link JPQLQueryContext} is used to query information about the + * application metadata and * cached information. */ final JPQLQueryContext queryContext; @@ -49,57 +54,94 @@ protected JPQLFunctionsAbstractBuilder(JPQLQueryContext queryContext) { } /** - * For Entity e with idAttr as a primary key: SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e + * For Entity e with idAttr as a primary key: + * SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e * * @param expression The {@link IdExpression} to visit */ @Override public void visit(IdExpression expression) { - //Fetch identification variable info + System.out.println("INSIDE VISIT *******"); IdentificationVariable identificationVariable = (IdentificationVariable) expression.getExpression(); String variableText = identificationVariable.getText(); String variableName = identificationVariable.getVariableName(); - - //Get id attribute name + // Get id attribute name ClassDescriptor descriptor = this.queryContext.getDeclaration(variableName).getDescriptor(); List primaryKeyFields = descriptor.getPrimaryKeyFields(); - String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyFields.get(0)); - StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression(expression.getParent(), variableText + "." + idAttributeName); - expression.setStateFieldPathExpression(stateFieldPathExpression); + // String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), + // primaryKeyFields.get(0)); + // StateFieldPathExpression stateFieldPathExpression = new + // StateFieldPathExpression(expression.getParent(), variableText + "." + + // idAttributeName); + // expression.setStateFieldPathExpression(stateFieldPathExpression); + // expression.getStateFieldPathExpression().accept(this); + if (!isEmbeddable(descriptor.getMappings())) { + for (DatabaseField primaryKeyField : primaryKeyFields) { + String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyField); + StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression( + expression.getParent(), variableText + "." + idAttributeName); + expression.setStateFieldPathExpression(stateFieldPathExpression); + // Continue with created StateFieldPathExpression + // It handle by ObjectBuilder booth @Id/primary key types (simple/composite) + expression.getStateFieldPathExpression().accept(this); + } + } else { + String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyFields.get(0)); + StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression( + expression.getParent(), variableText + "." + idAttributeName); + expression.setStateFieldPathExpression(stateFieldPathExpression); + // Continue with created StateFieldPathExpression + // It handle by ObjectBuilder booth @Id/primary key types (simple/composite) + expression.getStateFieldPathExpression().accept(this); - //Continue with created StateFieldPathExpression - //It handle by ObjectBuilder booth @Id/primary key types (simple/composite) - expression.getStateFieldPathExpression().accept(this); + } } /** - * For Entity e with versionAttr as a version attribute: SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e + * For Entity e with versionAttr as a version attribute: + * SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e * * @param expression The {@link VersionExpression} to visit */ @Override public void visit(VersionExpression expression) { - //Fetch identification variable info + // Fetch identification variable info IdentificationVariable identificationVariable = (IdentificationVariable) expression.getExpression(); String variableText = identificationVariable.getText(); String variableName = identificationVariable.getVariableName(); - //Get version attribute name + // Get version attribute name ClassDescriptor descriptor = this.queryContext.getDeclaration(variableName).getDescriptor(); - String versionAttributeName = ((VersionLockingPolicy) descriptor.getOptimisticLockingPolicy()).getVersionMapping().getAttributeName(); - StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression(expression.getParent(), variableText + "." + versionAttributeName); + String versionAttributeName = ((VersionLockingPolicy) descriptor.getOptimisticLockingPolicy()) + .getVersionMapping().getAttributeName(); + StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression(expression.getParent(), + variableText + "." + versionAttributeName); expression.setStateFieldPathExpression(stateFieldPathExpression); - //Continue with created StateFieldPathExpression + // Continue with created StateFieldPathExpression expression.getStateFieldPathExpression().accept(this); } private String getIdAttributeNameByField(List databaseMappings, DatabaseField field) { for (DatabaseMapping mapping : databaseMappings) { - if (field.equals(mapping.getField()) || mapping.isPrimaryKeyMapping()) { + if (mapping.getFields().size() > 1 && (field.equals(mapping.getField()) || mapping.isPrimaryKeyMapping())) { return mapping.getAttributeName(); + } else { + if ((field.equals(mapping.getField()) && mapping.isPrimaryKeyMapping())) { + return mapping.getAttributeName(); + } } } return null; } + + private boolean isEmbeddable(List databaseMappings) { + + for (DatabaseMapping databaseMapping : databaseMappings) { + if (databaseMapping.isPrimaryKeyMapping() && databaseMapping.getFields().size() > 1) { + return true; + } + } + return false; + } } diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java index 0e78b8e05d0..6d23c32ddd8 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java @@ -41,6 +41,7 @@ import org.eclipse.persistence.jpa.jpql.parser.EntryExpression; import org.eclipse.persistence.jpa.jpql.parser.ExtractExpression; import org.eclipse.persistence.jpa.jpql.parser.FunctionExpression; +import org.eclipse.persistence.jpa.jpql.parser.IdExpression; import org.eclipse.persistence.jpa.jpql.parser.IdentificationVariable; import org.eclipse.persistence.jpa.jpql.parser.IndexExpression; import org.eclipse.persistence.jpa.jpql.parser.Join; @@ -630,7 +631,7 @@ private void visitAbstractSelectClause(AbstractSelectClause expression) { multipleSelects = false; expression.getSelectExpression().accept(this); - if (multipleSelects) { + if (multipleSelects || (expression.getSelectExpression() instanceof IdExpression)) { query.returnWithoutReportQueryResult(); } else { From 9bf7da0eced2e5911340c45a4044bf5b30bf9c6b Mon Sep 17 00:00:00 2001 From: Ajay Paul Date: Fri, 8 Nov 2024 12:24:12 +0530 Subject: [PATCH 2/4] Added condition for embeddable --- .../jpql/JPQLFunctionsAbstractBuilder.java | 45 +++++++------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java index ac0dbb9f067..c96960a0573 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java @@ -28,14 +28,10 @@ import java.util.List; /** - * JPQL exclusive ID(), VERSION() functions/expressions are transformed there to - * StateFieldPathExpression. - * It should be used in the future for another JPQL functions/expressions which - * are not available at the DB level. - * E.g. For Entity e with idAttr as a primary key: - * SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e - * For Entity e with versionAttr as a version attribute: - * SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e + * JPQL exclusive ID(), VERSION() functions/expressions are transformed there to StateFieldPathExpression. + * It should be used in the future for another JPQL functions/expressions which are not available at the DB level. + * E.g. For Entity e with idAttr as a primary key: SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e + * For Entity e with versionAttr as a version attribute: SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e * * @author Radek Felcman * @since 5.0 @@ -43,8 +39,7 @@ public abstract class JPQLFunctionsAbstractBuilder extends EclipseLinkAnonymousExpressionVisitor { /** - * The {@link JPQLQueryContext} is used to query information about the - * application metadata and + * The {@link JPQLQueryContext} is used to query information about the application metadata and * cached information. */ final JPQLQueryContext queryContext; @@ -54,27 +49,20 @@ protected JPQLFunctionsAbstractBuilder(JPQLQueryContext queryContext) { } /** - * For Entity e with idAttr as a primary key: - * SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e + * For Entity e with idAttr as a primary key: SELECT ID(e) FROM Entity e -> SELECT e.idAttr FROM Entity e * * @param expression The {@link IdExpression} to visit */ @Override public void visit(IdExpression expression) { - System.out.println("INSIDE VISIT *******"); + //Fetch identification variable info IdentificationVariable identificationVariable = (IdentificationVariable) expression.getExpression(); String variableText = identificationVariable.getText(); String variableName = identificationVariable.getVariableName(); - // Get id attribute name + + //Get id attribute name ClassDescriptor descriptor = this.queryContext.getDeclaration(variableName).getDescriptor(); List primaryKeyFields = descriptor.getPrimaryKeyFields(); - // String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), - // primaryKeyFields.get(0)); - // StateFieldPathExpression stateFieldPathExpression = new - // StateFieldPathExpression(expression.getParent(), variableText + "." + - // idAttributeName); - // expression.setStateFieldPathExpression(stateFieldPathExpression); - // expression.getStateFieldPathExpression().accept(this); if (!isEmbeddable(descriptor.getMappings())) { for (DatabaseField primaryKeyField : primaryKeyFields) { String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyField); @@ -98,27 +86,24 @@ public void visit(IdExpression expression) { } /** - * For Entity e with versionAttr as a version attribute: - * SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e + * For Entity e with versionAttr as a version attribute: SELECT VERSION(e) FROM Entity e -> SELECT e.versionAttr FROM Entity e * * @param expression The {@link VersionExpression} to visit */ @Override public void visit(VersionExpression expression) { - // Fetch identification variable info + //Fetch identification variable info IdentificationVariable identificationVariable = (IdentificationVariable) expression.getExpression(); String variableText = identificationVariable.getText(); String variableName = identificationVariable.getVariableName(); - // Get version attribute name + //Get version attribute name ClassDescriptor descriptor = this.queryContext.getDeclaration(variableName).getDescriptor(); - String versionAttributeName = ((VersionLockingPolicy) descriptor.getOptimisticLockingPolicy()) - .getVersionMapping().getAttributeName(); - StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression(expression.getParent(), - variableText + "." + versionAttributeName); + String versionAttributeName = ((VersionLockingPolicy) descriptor.getOptimisticLockingPolicy()).getVersionMapping().getAttributeName(); + StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression(expression.getParent(), variableText + "." + versionAttributeName); expression.setStateFieldPathExpression(stateFieldPathExpression); - // Continue with created StateFieldPathExpression + //Continue with created StateFieldPathExpression expression.getStateFieldPathExpression().accept(this); } From 8e4a6d411c8d0d11732d9c6f5d990e7716e9be28 Mon Sep 17 00:00:00 2001 From: Ajay Paul Date: Wed, 22 Jan 2025 21:24:56 +0530 Subject: [PATCH 3/4] Added unit test case --- .../JUnitJPQLComplexAggregateTest.java | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java b/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java index f87ec1c69a4..eea1054e626 100644 --- a/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java +++ b/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java @@ -24,6 +24,7 @@ import org.eclipse.persistence.testing.models.jpa.advanced.compositepk.CompositePKTableCreator; import org.eclipse.persistence.testing.models.jpa.advanced.compositepk.Cubicle; import org.eclipse.persistence.testing.models.jpa.advanced.compositepk.Scientist; +import org.eclipse.persistence.testing.models.jpa.advanced.compositepk.ScientistPK; import org.junit.Assert; import java.util.Arrays; @@ -60,7 +61,7 @@ public static Test suite() { suite.addTest(new JUnitJPQLComplexAggregateTest("testSetup")); suite.addTest(new JUnitJPQLComplexAggregateTest("complexCountOnJoinedCompositePK")); - + suite.addTest(new JUnitJPQLComplexAggregateTest("testCompositePrimaryKey")); return suite; } @@ -121,4 +122,53 @@ public void complexCountOnJoinedCompositePK() { } } + public void testCompositePrimaryKey() { + EntityManager em = createEntityManager(); + try { + // Start the transaction + beginTransaction(em); + + // Create and set up the Scientist and Cubicle entities + Scientist scientist = new Scientist(); + scientist.setFirstName("John"); + scientist.setLastName("Doe"); + + Cubicle cubicle = new Cubicle(); + cubicle.setCode("G"); + cubicle.setScientist(scientist); + + scientist.setCubicle(cubicle); + + // Persist the entities + em.persist(cubicle); + em.persist(scientist); + em.flush(); + + // Execute the query to retrieve the primary keys + List keys = em.createQuery( + "SELECT ID(THIS) FROM Scientist WHERE firstName = :firstName ORDER BY idNumber", + ScientistPK.class + ) + .setParameter("firstName", "John") + .getResultList(); + + // Ensure the result size is greater than 0 + assertTrue("The result size should be greater than 0", !keys.isEmpty()); + + } catch (Exception e) { + // If there's an exception, rollback the transaction + rollbackTransaction(em); + throw e; + } finally { + // Ensure the transaction is rolled back if not committed + if (em.getTransaction().isActive()) { + rollbackTransaction(em); + } + // Close the entity manager + em.close(); + } +} + + + } From 5141e5ca5ad1afe8d7fd19c549e5965d1f30a1ee Mon Sep 17 00:00:00 2001 From: Ajay Paul Date: Thu, 27 Feb 2025 12:19:02 +0530 Subject: [PATCH 4/4] Review comments updated --- .../jpql/JPQLFunctionsAbstractBuilder.java | 19 +++++++++---------- .../internal/jpa/jpql/ReportItemBuilder.java | 6 +++++- .../JUnitJPQLComplexAggregateTest.java | 3 ++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java index c96960a0573..0ff9b8bef92 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/JPQLFunctionsAbstractBuilder.java @@ -63,7 +63,15 @@ public void visit(IdExpression expression) { //Get id attribute name ClassDescriptor descriptor = this.queryContext.getDeclaration(variableName).getDescriptor(); List primaryKeyFields = descriptor.getPrimaryKeyFields(); - if (!isEmbeddable(descriptor.getMappings())) { + if (isEmbeddable(descriptor.getMappings())) { + String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyFields.get(0)); + StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression( + expression.getParent(), variableText + "." + idAttributeName); + expression.setStateFieldPathExpression(stateFieldPathExpression); + // Continue with created StateFieldPathExpression + // It handle by ObjectBuilder booth @Id/primary key types (simple/composite) + expression.getStateFieldPathExpression().accept(this); + } else { for (DatabaseField primaryKeyField : primaryKeyFields) { String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyField); StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression( @@ -73,15 +81,6 @@ public void visit(IdExpression expression) { // It handle by ObjectBuilder booth @Id/primary key types (simple/composite) expression.getStateFieldPathExpression().accept(this); } - } else { - String idAttributeName = getIdAttributeNameByField(descriptor.getMappings(), primaryKeyFields.get(0)); - StateFieldPathExpression stateFieldPathExpression = new StateFieldPathExpression( - expression.getParent(), variableText + "." + idAttributeName); - expression.setStateFieldPathExpression(stateFieldPathExpression); - // Continue with created StateFieldPathExpression - // It handle by ObjectBuilder booth @Id/primary key types (simple/composite) - expression.getStateFieldPathExpression().accept(this); - } } diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java index 6d23c32ddd8..d477aae97ee 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/internal/jpa/jpql/ReportItemBuilder.java @@ -626,12 +626,16 @@ public void visit(ValueExpression expression) { addAttribute(identificationVariable.getText(), queryExpression); } + @Override + public void visit(IdExpression expression){ + multipleSelects = true; + } private void visitAbstractSelectClause(AbstractSelectClause expression) { multipleSelects = false; expression.getSelectExpression().accept(this); - if (multipleSelects || (expression.getSelectExpression() instanceof IdExpression)) { + if (multipleSelects) { query.returnWithoutReportQueryResult(); } else { diff --git a/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java b/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java index eea1054e626..a28792686ae 100644 --- a/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java +++ b/jpa/eclipselink.jpa.testapps/jpa.test.jpql/src/test/java/org/eclipse/persistence/testing/tests/jpa/jpql/advanced/compositepk/JUnitJPQLComplexAggregateTest.java @@ -154,7 +154,8 @@ public void testCompositePrimaryKey() { // Ensure the result size is greater than 0 assertTrue("The result size should be greater than 0", !keys.isEmpty()); - + Assert.assertEquals("first name should be John", "John", keys.get(0).firstName); + Assert.assertEquals("last name should be John", "Doe", keys.get(0).lastName); } catch (Exception e) { // If there's an exception, rollback the transaction rollbackTransaction(em);