diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java index 9e983da52..5ec18e919 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java @@ -489,7 +489,14 @@ public class JPQLExpressionBuilder for (int i = 0; i < groupByCount; i++) { JPQLNode node = groupByNode.getChild(i); - exps.grouping[i] = getValue(node); + Value val = getValue(node); + if (val instanceof Path) { + FieldMetaData fmd = ((Path) val).last(); + if (fmd != null && fmd.getValue().getTypeMetaData() != null && fmd.getValue().isEmbedded()) + throw parseException(EX_USER, "cant-groupby-embeddable", + new Object[]{ node.getChildCount() > 1 ? assemble(node) : node.text }, null); + } + exps.grouping[i] = val; } } @@ -1166,7 +1173,7 @@ public class JPQLExpressionBuilder case JJTGENERALIDENTIFIER: // KEY(e), VALUE(e) - if (node.parent.parent.id == JJTWHERE) + if (node.parent.parent.id == JJTWHERE || node.parent.id == JJTGROUPBY) return getGeneralIdentifier(onlyChild(node), true); return getQualifiedIdentifier(onlyChild(node)); @@ -1729,7 +1736,7 @@ public class JPQLExpressionBuilder return path; } - private Value getGeneralIdentifier(JPQLNode node, boolean inWhereClause) { + private Value getGeneralIdentifier(JPQLNode node, boolean verifyEmbeddable) { JPQLNode id = onlyChild(node); Path path = validateMapPath(node, id); @@ -1737,14 +1744,18 @@ public class JPQLExpressionBuilder path = (Path) factory.getKey(path); FieldMetaData fld = path.last(); ClassMetaData meta = fld.getKey().getTypeMetaData(); - if (inWhereClause && + if (verifyEmbeddable && (node.id == JJTKEY && meta != null && fld.getKey().isEmbedded()) || (node.id == JJTVALUE && fld.isElementCollection() && - fld.getElement().getEmbeddedMetaData() != null)) + fld.getElement().getEmbeddedMetaData() != null)) { // check basic type + if (node.parent.parent.id == JJTGROUPBY) + throw parseException(EX_USER, "cant-groupby-key-value-embeddable", + new Object[]{ node.id == JJTVALUE ? "VALUE" : "KEY", id.text }, null); + else throw parseException(EX_USER, "bad-general-identifier", new Object[]{ node.id == JJTVALUE ? "VALUE" : "KEY", id.text }, null); - + } return path; } diff --git a/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt b/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt index 3db39989c..d2a7adfaa 100644 --- a/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt +++ b/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt @@ -673,6 +673,7 @@ void groupby_item() : { } LOOKAHEAD(path()) path() | LOOKAHEAD(groupby_extension()) groupby_extension() | LOOKAHEAD(qualified_path()) qualified_path() + | LOOKAHEAD(general_identification_variable()) general_identification_variable() | LOOKAHEAD(identification_variable()) identification_variable() } diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties index 3e47631ed..9e00e4e2e 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties @@ -80,6 +80,7 @@ bad-general-identifier: The "{0}({1})" is an embeddable which not allowed \ in conditional expression. bad-predicate: JPQL query does not support conditional expression over \ embeddable class. JPQL string: "{0}". -cant-bulk-update-embeddable: Bulk update of embeddable field is not allowed: \ - "{0}". +cant-bulk-update-embeddable: Bulk update of embeddables: "{0}" is not allowed. +cant-groupby-embeddable: Grouping by embeddables: "{0}" is not allowed. +cant-groupby-key-value-embeddable: Grouping by embeddables: "{0}({1})" is not allowed. no-constructor: NEW constructor operation could not resolve class named "{0}". diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/embed/TestEmbeddable.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/embed/TestEmbeddable.java index 54bc47520..f0f971128 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/embed/TestEmbeddable.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/embed/TestEmbeddable.java @@ -106,6 +106,24 @@ public class TestEmbeddable extends SQLListenerTestCase { } } + public void testGroupByEmbed() { + EntityManager em = emf.createEntityManager(); + String query[] = { + "select KEY(e) from Department3 d join d.emps e group by KEY(e)", + "select a.embed from EntityA_Embed_Embed a group by a.embed", + "select e from EntityA_Embed_Embed a join a.embed e group by e", + }; + List rs = null; + for (int i = 0; i < query.length; i++) { + try { + rs = em.createQuery(query[i]).getResultList(); + } catch(ArgumentException e) { + System.out.println(e.getMessage()); // as expected : Group by embeddable field is not allowed + } + } + em.close(); + } + public void testKeyEmbeddableCompare() { EntityManager em = emf.createEntityManager(); String query[] = {