Skip to content

Commit

Permalink
Fix search by tag count queries for commands, clusters and applications
Browse files Browse the repository at this point in the history 
Searching by tags for these entites results in a query using `group by` and `having`. This was working fine for the actual
content queries but the count queries were getting messed up because it couldn't count the result properly as it was just
and array of 1's being return (counting the group size and all were one). It was actually the length of the result set we'd
have wanted for the total count. This wouldn't work though for the ones were tags weren't involved because that query doesn't
have a group by and a having.

This change modifies the count queries to instead of trying to count directly with the same where predicate it actually
counts the results of the real query as a subquery (all on database side so it doesn't return all the data). This way it
doesn't matter if there's a `group by` or not. Now the total results returned via the API in the `Page` object are correct and
pagination can continue properly.
  • Loading branch information
@tgianos
tgianos committed Mar 25, 2022
1 parent 9304981 commit 04e7ac0
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 40 deletions.
68 changes: 31 additions & 37 deletions ...src/main/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImpl.java
View file
Expand Up @@ -125,6 +125,7 @@
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Order;
import javax.persistence.criteria.Root;
import javax.persistence.criteria.Subquery;
import javax.validation.ConstraintViolationException;
import javax.validation.Valid;
import javax.validation.constraints.Min;
Expand Down Expand Up @@ -313,11 +314,13 @@ public Page<Application> findApplications(
final CriteriaBuilder criteriaBuilder = this.entityManager.getCriteriaBuilder();
final CriteriaQuery<Long> countQuery = criteriaBuilder.createQuery(Long.class);
final Root<ApplicationEntity> countQueryRoot = countQuery.from(ApplicationEntity.class);
countQuery.select(criteriaBuilder.count(countQueryRoot));
countQuery.where(
final Subquery<Long> countIdSubQuery = countQuery.subquery(Long.class);
final Root<ApplicationEntity> countIdSubQueryRoot = countIdSubQuery.from(ApplicationEntity.class);
countIdSubQuery.select(countIdSubQueryRoot.get(ApplicationEntity_.id));
countIdSubQuery.where(
ApplicationPredicates.find(
countQueryRoot,
countQuery,
countIdSubQueryRoot,
countIdSubQuery,
criteriaBuilder,
name,
user,
Expand All @@ -326,15 +329,11 @@ public Page<Application> findApplications(
type
)
);
countQuery.select(criteriaBuilder.count(countQueryRoot));
countQuery.where(countQueryRoot.get(ApplicationEntity_.id).in(countIdSubQuery));

final List<Long> applicationsCount = this.entityManager.createQuery(countQuery).getResultList();
if (applicationsCount.isEmpty()) {
// SELECT COUNT ... GROUP BY ... HAVING ... may return NULL
return new PageImpl<>(new ArrayList<>(0));
}

final Long totalCount = applicationsCount.get(0);
if (totalCount == 0) {
final Long totalCount = this.entityManager.createQuery(countQuery).getSingleResult();
if (totalCount == null || totalCount == 0) {
// short circuit for no results
return new PageImpl<>(new ArrayList<>(0));
}
Expand Down Expand Up @@ -552,11 +551,13 @@ public Page<Cluster> findClusters(
final CriteriaBuilder criteriaBuilder = this.entityManager.getCriteriaBuilder();
final CriteriaQuery<Long> countQuery = criteriaBuilder.createQuery(Long.class);
final Root<ClusterEntity> countQueryRoot = countQuery.from(ClusterEntity.class);
countQuery.select(criteriaBuilder.count(countQueryRoot));
countQuery.where(
final Subquery<Long> countIdSubQuery = countQuery.subquery(Long.class);
final Root<ClusterEntity> countIdSubQueryRoot = countIdSubQuery.from(ClusterEntity.class);
countIdSubQuery.select(countIdSubQueryRoot.get(ClusterEntity_.id));
countIdSubQuery.where(
ClusterPredicates.find(
countQueryRoot,
countQuery,
countIdSubQueryRoot,
countIdSubQuery,
criteriaBuilder,
name,
statusStrings,
Expand All @@ -565,15 +566,11 @@ public Page<Cluster> findClusters(
maxUpdateTime
)
);
countQuery.select(criteriaBuilder.count(countQueryRoot));
countQuery.where(countQueryRoot.get(ClusterEntity_.id).in(countIdSubQuery));

final List<Long> clustersCount = this.entityManager.createQuery(countQuery).getResultList();
if (clustersCount.isEmpty()) {
// SELECT COUNT ... GROUP BY ... HAVING ... may return NULL
return new PageImpl<>(new ArrayList<>(0));
}

final Long totalCount = clustersCount.get(0);
if (totalCount == 0) {
final Long totalCount = this.entityManager.createQuery(countQuery).getSingleResult();
if (totalCount == null || totalCount == 0) {
// short circuit for no results
return new PageImpl<>(new ArrayList<>(0));
}
Expand Down Expand Up @@ -860,28 +857,25 @@ public Page<Command> findCommands(
final CriteriaBuilder criteriaBuilder = this.entityManager.getCriteriaBuilder();
final CriteriaQuery<Long> countQuery = criteriaBuilder.createQuery(Long.class);
final Root<CommandEntity> countQueryRoot = countQuery.from(CommandEntity.class);
countQuery.select(criteriaBuilder.count(countQueryRoot));
countQuery.where(
final Subquery<Long> countIdSubQuery = countQuery.subquery(Long.class);
final Root<CommandEntity> countIdSubQueryRoot = countIdSubQuery.from(CommandEntity.class);
countIdSubQuery.select(countIdSubQueryRoot.get(CommandEntity_.id));
countIdSubQuery.where(
CommandPredicates.find(
countQueryRoot,
countQuery,
countIdSubQueryRoot,
countIdSubQuery,
criteriaBuilder,
name,
user,
statusStrings,
tagEntities
)
);
countQuery.select(criteriaBuilder.count(countQueryRoot));
countQuery.where(countQueryRoot.get(CommandEntity_.id).in(countIdSubQuery));

final List<Long> commandsCount = this.entityManager.createQuery(countQuery).getResultList();

if (commandsCount.isEmpty()) {
// SELECT COUNT ... GROUP BY ... HAVING ... may return NULL
return new PageImpl<>(new ArrayList<>(0));
}

final Long totalCount = commandsCount.get(0);
if (totalCount == 0) {
final Long totalCount = this.entityManager.createQuery(countQuery).getSingleResult();
if (totalCount == null || totalCount == 0) {
// short circuit for no results
return new PageImpl<>(new ArrayList<>(0));
}
Expand Down
3 changes: 2 additions & 1 deletion ...om/netflix/genie/web/data/services/impl/jpa/queries/predicates/ApplicationPredicates.java
View file
Expand Up @@ -21,6 +21,7 @@
import org.apache.commons.lang3.StringUtils;

import javax.annotation.Nullable;
import javax.persistence.criteria.AbstractQuery;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Join;
Expand Down Expand Up @@ -58,7 +59,7 @@ private ApplicationPredicates() {
*/
public static Predicate find(
final Root<ApplicationEntity> root,
final CriteriaQuery<?> cq,
final AbstractQuery<?> cq,
final CriteriaBuilder cb,
@Nullable final String name,
@Nullable final String user,
Expand Down
3 changes: 2 additions & 1 deletion ...va/com/netflix/genie/web/data/services/impl/jpa/queries/predicates/ClusterPredicates.java
View file
Expand Up @@ -24,6 +24,7 @@
import org.springframework.data.jpa.domain.Specification;

import javax.annotation.Nullable;
import javax.persistence.criteria.AbstractQuery;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Join;
Expand Down Expand Up @@ -64,7 +65,7 @@ private ClusterPredicates() {
*/
public static Predicate find(
final Root<ClusterEntity> root,
final CriteriaQuery<?> cq,
final AbstractQuery<?> cq,
final CriteriaBuilder cb,
@Nullable final String name,
@Nullable final Set<String> statuses,
Expand Down
3 changes: 2 additions & 1 deletion ...va/com/netflix/genie/web/data/services/impl/jpa/queries/predicates/CommandPredicates.java
View file
Expand Up @@ -22,6 +22,7 @@
import org.apache.commons.lang3.StringUtils;

import javax.annotation.Nullable;
import javax.persistence.criteria.AbstractQuery;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.Join;
Expand Down Expand Up @@ -60,7 +61,7 @@ private CommandPredicates() {
*/
public static Predicate find(
final Root<CommandEntity> root,
final CriteriaQuery<?> cq,
final AbstractQuery<?> cq,
final CriteriaBuilder cb,
@Nullable final String name,
@Nullable final String user,
Expand Down

0 comments on commit 04e7ac0

Please sign in to comment.