From 1a216e9fb71736bbcf2007c0e3a4463332280eeb Mon Sep 17 00:00:00 2001 From: Fay Wang Date: Tue, 2 Feb 2010 07:58:31 +0000 Subject: [PATCH] OPENJPA-1483: support count distinct compound key git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@905540 13f79535-47bb-0310-9956-ffa450edef68 --- .../openjpa/jdbc/kernel/exps/Count.java | 32 ++++++++++-- .../openjpa/jdbc/kernel/exps/Distinct.java | 18 +++++++ .../openjpa/jdbc/kernel/exps/PCPath.java | 14 ++++++ .../jdbc/kernel/exps/SelectConstructor.java | 42 ++++++++++++++-- .../openjpa/jdbc/kernel/exps/UnaryOp.java | 4 ++ .../apache/openjpa/jdbc/sql/SelectImpl.java | 7 +-- .../jdbc/kernel/exps/localizer.properties | 6 ++- .../enhance/identity/TestMappedById.java | 49 +++++++++++++++++++ 8 files changed, 160 insertions(+), 12 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java index e58ffbbf0..023d2dc91 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Count.java @@ -18,6 +18,7 @@ */ package org.apache.openjpa.jdbc.kernel.exps; +import org.apache.openjpa.jdbc.schema.Column; import org.apache.openjpa.jdbc.sql.SQLBuffer; import org.apache.openjpa.jdbc.sql.Select; @@ -29,16 +30,30 @@ import org.apache.openjpa.jdbc.sql.Select; class Count extends UnaryOp { + private boolean isCountMultiColumns = false; + private boolean isCountDistinct = false; + /** * Constructor. Provide the value to operate on. */ public Count(Val val) { super(val); + if (val instanceof Distinct) + isCountDistinct = true; } public ExpState initialize(Select sel, ExpContext ctx, int flags) { // join into related object if present - return initializeValue(sel, ctx, JOIN_REL); + ExpState expState = initializeValue(sel, ctx, JOIN_REL); + Val val = isCountDistinct ? ((Distinct)getValue()).getValue() : getValue(); + if (val instanceof PCPath) { + Column[] cols = ((PCPath)val).getColumns(expState); + if (cols.length > 1) { + isCountMultiColumns = true; + } + } + + return expState; } protected Class getType(Class c) { @@ -52,15 +67,24 @@ class Count public boolean isAggregate() { return true; } - + + public boolean isCountDistinctMultiCols() { + return isCountDistinct && isCountMultiColumns; + } + /** * Overrides SQL formation by replacing COUNT(column) by COUNT(*) when specific conditions are met and * DBDictionary configuration useWildCardForCount is set. */ @Override public void appendTo(Select sel, ExpContext ctx, ExpState state, SQLBuffer sql, int index) { - super.appendTo(sel, ctx, state, sql, index); - if (ctx.store.getDBDictionary().useWildCardForCount && state.joins.isEmpty()) { + if (isCountDistinctMultiCols()) { + getValue().appendTo(sel, ctx, state, sql, 0); + sql.addCastForParam(getOperator(), getValue()); + } else + super.appendTo(sel, ctx, state, sql, index); + if ((ctx.store.getDBDictionary().useWildCardForCount && state.joins.isEmpty()) || + !isCountDistinct && isCountMultiColumns){ String s = sql.getSQL(); if (s.startsWith("COUNT(") && s.endsWith(")")) { sql.replaceSqlString("COUNT(".length(), s.length()-1, "*"); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java index 36f4ec712..486a940c7 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/Distinct.java @@ -41,4 +41,22 @@ class Distinct protected String getOperator() { return "DISTINCT"; } + + @Override + public void appendTo(Select sel, ExpContext ctx, ExpState state, + SQLBuffer sql, int index) { + Val val = getValue(); + if (val instanceof PCPath) { + boolean noParen = getNoParen(); + sql.append(getOperator()); + sql.append(noParen ? " " : "("); + ((PCPath)val).appendTo(sel, ctx, state, sql); + sql.addCastForParam(getOperator(), val); + if (!noParen) + sql.append(")"); + + } else + super.appendTo(sel, ctx, state, sql, index); + } + } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java index e22509dd9..ea59e7836 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/PCPath.java @@ -970,9 +970,23 @@ public class PCPath return getColumns(state).length; } + public void appendTo(Select sel, ExpContext ctx, ExpState state, + SQLBuffer sql) { + Column[] cols = getColumns(state); + for (int i = 0; i < cols.length; i++) { + appendTo(sel, state, sql, cols[i]); + if (i < cols.length -1) + sql.append(", "); + } + } + public void appendTo(Select sel, ExpContext ctx, ExpState state, SQLBuffer sql, int index) { Column col = getColumns(state)[index]; + appendTo(sel, state, sql, col); + } + + public void appendTo(Select sel, ExpState state, SQLBuffer sql, Column col) { if (sel != null) sel.setSchemaAlias(_schemaAlias); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java index 179e2dfa5..204294a0c 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/SelectConstructor.java @@ -36,6 +36,8 @@ import org.apache.openjpa.kernel.exps.Expression; import org.apache.openjpa.kernel.exps.QueryExpressions; import org.apache.openjpa.kernel.exps.Subquery; import org.apache.openjpa.kernel.exps.Value; +import org.apache.openjpa.lib.util.Localizer; +import org.apache.openjpa.util.UnsupportedException; /** * Turns parsed queries into selects. @@ -48,6 +50,7 @@ public class SelectConstructor private boolean _extent = false; private Select _subselect = null; + private static final Localizer _loc = Localizer.forPackage(SelectConstructor.class); /** * Return true if we know the select to have on criteria; to be an extent. @@ -115,6 +118,18 @@ public class SelectConstructor } for (int i = 0; i < exps.grouping.length; i++) ((Val) exps.grouping[i]).groupBy(sel, ctx, state.grouping[i]); + + if (exps.projections.length == 1) { + Val val = (Val) exps.projections[0]; + if (val instanceof Count && ((Count)val).isCountDistinctMultiCols()) { + Select newSel = ctx.store.getSQLFactory().newSelect(); + newSel.select("COUNT(*)", val); + newSel.setExpectedResultCount(1, true); + newSel.setFromSelect(sel); + sel.setExpectedResultCount(0, true); + sel = newSel; + } + } return sel; } @@ -235,6 +250,10 @@ public class SelectConstructor // projections; this ensures that we have all our joins cached state.projections[i] = resultVal.initialize(sel, ctx, Val.JOIN_REL | Val.FORCE_OUTER); + if (exps.projections.length > 1 && resultVal instanceof Count) { + if (((Count)resultVal).isCountDistinctMultiCols()) + throw new UnsupportedException(_loc.get("count-distinct-multi-col-only")); + } joins = sel.and(joins, state.projections[i].joins); } } @@ -290,6 +309,8 @@ public class SelectConstructor Select inner = sel.getFromSelect(); Val val; Joins joins = null; + boolean isCountDistinctMultiCols = false; + if (sel.getSubselectPath() != null) joins = sel.newJoins().setSubselect(sel.getSubselectPath()); @@ -307,9 +328,18 @@ public class SelectConstructor // subselect for objects; we really just need the primary key values sel.select(mapping.getPrimaryKeyColumns(), joins); } else { + if (exps.projections.length == 1) { + val = (Val) exps.projections[0]; + if (val instanceof Count && ((Count)val).isCountDistinctMultiCols()) { + isCountDistinctMultiCols = true; + if (sel.getParent() != null) + throw new UnsupportedException(_loc.get("count-distinct-multi-col-subselect-unsupported")); + } + } + // if we have an inner select, we need to select the candidate // class' pk columns to guarantee unique instances - if (inner != null) + if (inner != null && !isCountDistinctMultiCols) inner.select(mapping.getPrimaryKeyColumns(), joins); // select each result value; no need to pass on the eager mode since @@ -317,9 +347,13 @@ public class SelectConstructor boolean pks = sel.getParent() != null; for (int i = 0; i < exps.projections.length; i++) { val = (Val) exps.projections[i]; - if (inner != null) - val.selectColumns(inner, ctx, state.projections[i], pks); - val.select(sel, ctx, state.projections[i], pks); + if (inner != null) { + if (!isCountDistinctMultiCols) + val.selectColumns(inner, ctx, state.projections[i], pks); + else + val.select(inner, ctx, state.projections[i], pks); + } else + val.select(sel, ctx, state.projections[i], pks); } // make sure having columns are selected since it is required by diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java index b7246ba20..be543b648 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/UnaryOp.java @@ -74,6 +74,10 @@ abstract class UnaryOp public void setImplicitType(Class type) { _cast = type; } + + public boolean getNoParen() { + return _noParen; + } public ExpState initialize(Select sel, ExpContext ctx, int flags) { return initializeValue(sel, ctx, flags); diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java index 1e5dc54c9..d3d033316 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/SelectImpl.java @@ -3111,9 +3111,10 @@ public class SelectImpl alias = alias + _dict.getStringVal; String as = null; - if (inner) - as = ((String) alias).replace('.', '_'); - else if (_selectAs != null) + if (inner) { + if (alias instanceof String) + as = ((String) alias).replace('.', '_'); + } else if (_selectAs != null) as = (String) _selectAs.get(id); else if (id instanceof Value) as = ((Value) id).getAlias(); diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties index 74f24f6f7..70fa2dd8a 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties +++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/kernel/exps/localizer.properties @@ -30,4 +30,8 @@ invalid-unbound-var: Invalid unbound variable "{0}" in query. no-order-column: Field "{0}" does not have order column defined". not-collection-parm: Invalid input parameter "{0}", a collection-valued \ input parameter is expected. -empty-collection-parm: Input parameter "{0}" is empty. \ No newline at end of file +empty-collection-parm: Input parameter "{0}" is empty. +count-distinct-multi-col-only: Count distinct compound primary key is not \ + supported when there are other projection items. +count-distinct-multi-col-subselect-unsupported: Count distinct multiple columns \ + in the subselect is not supported. \ No newline at end of file diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMappedById.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMappedById.java index 844ab6e13..419f2d06d 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMappedById.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/enhance/identity/TestMappedById.java @@ -206,6 +206,55 @@ public class TestMappedById extends SingleEMFTestCase { assertNotNull(newDep); } + public void testCountDistinctMultiCols() { + EntityManager em = emf.createEntityManager(); + + Employee2 emp1 = new Employee2(); + EmployeeId2 empId1 = new EmployeeId2(); + empId1.setFirstName("James"); + empId1.setLastName("Bond"); + emp1.setEmpId(empId1); + + Employee2 emp2 = new Employee2(); + EmployeeId2 empId2 = new EmployeeId2(); + empId2.setFirstName("James"); + empId2.setLastName("Obama"); + emp2.setEmpId(empId2); + + Dependent2 dep1 = new Dependent2(); + DependentId2 depId1 = new DependentId2(); + depId1.setEmpPK(empId1); + depId1.setName("Alan"); + dep1.setId(depId1); + + Dependent2 dep2 = new Dependent2(); + DependentId2 depId2 = new DependentId2(); + depId2.setEmpPK(empId2); + depId2.setName("Darren"); + dep2.setId(depId2); + + em.persist(emp1); + em.persist(emp2); + em.persist(dep1); + em.persist(dep2); + + em.getTransaction().begin(); + em.flush(); + em.getTransaction().commit(); + + String[] jpqls = { + "SELECT COUNT (DISTINCT d2.emp) FROM Dependent2 d2", + "select count (DISTINCT d2) from Dependent2 d2", + }; + + for (int i = 0; i < jpqls.length; i++) { + Query q = em.createQuery(jpqls[i]) ; + Long o = (Long)q.getSingleResult(); + int count = (int)o.longValue(); + assertEquals(2, count); + } + } + public void createObj1() { EntityManager em = emf.createEntityManager(); EntityTransaction tran = em.getTransaction();