cayenne-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ntimof...@apache.org
Subject cayenne git commit: Explicitly add columns used in query to GROUP BY clause when HAVING clause is used
Date Wed, 03 May 2017 15:13:59 GMT
Repository: cayenne
Updated Branches:
  refs/heads/master a2e0452d6 -> a20830220


Explicitly add columns used in query to GROUP BY clause
when HAVING clause is used


Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo
Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/a2083022
Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/a2083022
Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/a2083022

Branch: refs/heads/master
Commit: a20830220b62a3f58cba16cc4d0a1225d684f08f
Parents: a2e0452
Author: Nikita Timofeev <stariy95@gmail.com>
Authored: Wed May 3 18:13:54 2017 +0300
Committer: Nikita Timofeev <stariy95@gmail.com>
Committed: Wed May 3 18:13:54 2017 +0300

----------------------------------------------------------------------
 .../select/DefaultSelectTranslator.java         | 35 ++++++++------
 .../apache/cayenne/query/ColumnSelectIT.java    | 49 ++++++++++++++++++++
 2 files changed, 69 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cayenne/blob/a2083022/cayenne-server/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java
b/cayenne-server/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java
index 76af339..ea0ea53 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java
@@ -142,6 +142,22 @@ public class DefaultSelectTranslator extends QueryAssembler implements
SelectTra
 		QualifierTranslator qualifierTranslator = adapter.getQualifierTranslator(this);
 		StringBuilder whereQualifierBuffer = qualifierTranslator.appendPart(new StringBuilder());
 
+		// build having qualifier
+		Expression havingQualifier = ((SelectQuery)query).getHavingQualifier();
+		StringBuilder havingQualifierBuffer = null;
+		if(havingQualifier != null) {
+			haveAggregate = true;
+			QualifierTranslator havingQualifierTranslator = adapter.getQualifierTranslator(this);
+			havingQualifierTranslator.setQualifier(havingQualifier);
+			havingQualifierBuffer = havingQualifierTranslator.appendPart(new StringBuilder());
+		}
+
+		if(!haveAggregate && groupByColumns != null) {
+			// if no expression with aggregation function found
+			// in select columns and there is no having clause
+			groupByColumns.clear();
+		}
+
 		// build ORDER BY
 		OrderingTranslator orderingTranslator = new OrderingTranslator(this);
 		StringBuilder orderingBuffer = orderingTranslator.appendPart(new StringBuilder());
@@ -209,21 +225,15 @@ public class DefaultSelectTranslator extends QueryAssembler implements
SelectTra
 			queryBuf.append(whereQualifierBuffer);
 		}
 
-		if(haveAggregate && !groupByColumns.isEmpty()) {
+		if(groupByColumns != null && !groupByColumns.isEmpty()) {
 			queryBuf.append(" GROUP BY ");
 			appendGroupByColumns(queryBuf, groupByColumns);
 		}
 
 		// append HAVING qualifier
-		QualifierTranslator havingQualifierTranslator = adapter.getQualifierTranslator(this);
-		Expression havingQualifier = ((SelectQuery)query).getHavingQualifier();
-		if(havingQualifier != null) {
-			havingQualifierTranslator.setQualifier(havingQualifier);
-			StringBuilder havingQualifierBuffer = havingQualifierTranslator.appendPart(new StringBuilder());
-			if(havingQualifierBuffer.length() > 0) {
-				queryBuf.append(" HAVING ");
-				queryBuf.append(havingQualifierBuffer);
-			}
+		if(havingQualifierBuffer != null && havingQualifierBuffer.length() > 0) {
+			queryBuf.append(" HAVING ");
+			queryBuf.append(havingQualifierBuffer);
 		}
 
 		// append prebuilt ordering
@@ -468,11 +478,6 @@ public class DefaultSelectTranslator extends QueryAssembler implements
SelectTra
 		qualifierTranslator.setForceJoinForRelations(false);
 		joinListener = null;
 
-		if(!haveAggregate) {
-			// if no expression with aggregation function found, we don't need this information
-			groupByColumns.clear();
-		}
-
 		return columns;
 	}
 

http://git-wip-us.apache.org/repos/asf/cayenne/blob/a2083022/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
----------------------------------------------------------------------
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java b/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
index 2d79d29..3d8668f 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/query/ColumnSelectIT.java
@@ -208,6 +208,55 @@ public class ColumnSelectIT extends ServerCase {
     }
 
     @Test
+    public void testHavingWithoutAggregate() throws Exception {
+        Object date = ObjectSelect.columnQuery(Artist.class, Artist.DATE_OF_BIRTH, Artist.ARTIST_NAME)
+                .having(Artist.ARTIST_NAME.like("a%"))
+                .selectFirst(context);
+        assertNotNull(date);
+    }
+
+    /**
+     * This test will fail as ARTIST_NAME wouldn't be in GROUP BY,
+     * but potentially we can detect this case (e.g. add all fields in HAVING clause to GROUP
BY).
+     * This just doesn't seem right as in this case WHERE a better choice.
+     *
+     * Current workaround for this is the method above, i.e. just adding field used
+     * in a HAVING qualifier into select.
+     */
+    @Ignore
+    @Test
+    public void testHavingWithoutSelect() throws Exception {
+        Object date = ObjectSelect.columnQuery(Artist.class, Artist.DATE_OF_BIRTH)
+                .having(Artist.ARTIST_NAME.like("a%"))
+                .selectFirst(context);
+        assertNotNull(date);
+    }
+
+    /**
+     * Test using field in HAVING clause without using it in SELECT
+     * i.e. something like this:
+     *      SELECT a.name FROM artist a JOIN painting p ON (..) HAVING COUNT(p.id) > 4
+     */
+    @Test
+    public void testSelectRelationshipCountHavingWithoutFieldSelect() throws Exception {
+        Object[] result = null;
+        try {
+            result = ObjectSelect.query(Artist.class)
+                    .columns(Artist.ARTIST_NAME)
+                    .having(Artist.PAINTING_ARRAY.count().gt(4L))
+                    .selectOne(context);
+        } catch (CayenneRuntimeException ex) {
+            if(unitDbAdapter.supportsExpressionInHaving()) {
+                fail();
+            } else {
+                return;
+            }
+        }
+
+        assertEquals("artist2", result[0]);
+    }
+
+    @Test
     public void testSelectRelationshipCountHaving() throws Exception {
         Property<Long> paintingCount = Artist.PAINTING_ARRAY.count();
 


Mime
View raw message