From 5ce9f384c23539d89876a408c6ff43878cda0364 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Tue, 7 May 2024 13:22:02 -0500 Subject: [PATCH 1/3] Issue 1931: Pagination not working properly for combined filters list. This is a regression from the aggressive performance optimizations such as commit b2c538c5c9c7d99a18c9f8c2c47b8cc98c01a260. The case that is not correctly handled is where there are multiple separate field values being selected and filtered upon. This reverts the problematic optimization to ensure that the generated queries are correct. This has a cost of losing a significant performance optimization. The performance optimization will have to be re-looked at and a more correct approach without any regressions will need to be figured out. --- .../model/repo/impl/SubmissionRepoImpl.java | 114 +----------------- 1 file changed, 1 insertion(+), 113 deletions(-) diff --git a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java index 79fb337c0..26b36e5e6 100644 --- a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java +++ b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java @@ -429,16 +429,12 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { }); StringBuilder sqlSelectBuilder = new StringBuilder("SELECT DISTINCT "); - StringBuilder sqlCountSelectBuilder = new StringBuilder(); Map> sqlColumnsBuilders = new HashMap<>(); - Map> sqlCountWhereFilterBuilders = new HashMap<>(); - Map sqlCountWherePredicate = new HashMap<>(); List sqlAliasBuilders = new ArrayList<>(); StringBuilder sqlJoinsBuilder = new StringBuilder(); StringBuilder sqlBuilder; - StringBuilder sqlCountBuilder; StringBuilder sqlWheresExcludeBuilder = new StringBuilder(); StringBuilder sqlOrderBysBuilder = new StringBuilder(); @@ -486,13 +482,11 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = new StringBuilder(); - sqlCountBuilder = new StringBuilder(); switch (submissionListColumn.getInputType().getName()) { case "INPUT_DEGREEDATE": // Column's values are of type 'MMMM yyyy' (in SQL date format would be 'Month YYYY'). sqlBuilder.append("LOWER(pfv").append(n).append(".value) = LOWER('").append(filterString).append("')"); - sqlCountBuilder.append("LOWER(fv.value) = LOWER('").append(filterString).append("')"); break; case "INPUT_DATE": // Column's values are of type 'yyyy-mm-dd' as required by the SQL standard to represent a date without time. @@ -504,35 +498,20 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append(".value AS DATE) BETWEEN CAST('").append(dates[0]) .append("' AS DATE) AND CAST('").append(dates[1]) .append("' AS DATE)"); - sqlCountBuilder - .append("CAST(fv.value AS DATE) BETWEEN CAST('").append(dates[0]) - .append("' AS DATE) AND CAST('").append(dates[1]) - .append("' AS DATE)"); } else { // Date Match sqlBuilder.append("pfv").append(n).append(".value = '").append(filterString).append("'"); - sqlCountBuilder.append("fv.value = '").append(filterString).append("'"); } break; case "INPUT_CHECKBOX": sqlBuilder.append("pfv").append(n).append(".value = '").append(filterString).append("'"); - sqlCountBuilder.append("fv.value = '").append(filterString).append("'"); // Column's values are a boolean if (!Boolean.valueOf(filterString)) { sqlWhereBuilderList.add(sqlBuilder); - if (!sqlCountWherePredicate.containsKey(predicateId)) { - sqlCountWherePredicate.put(predicateId, new StringBuilder()); - } - - sqlCountWherePredicate.get(predicateId).append(" (").append(sqlCountBuilder).append(") OR"); - sqlBuilder = new StringBuilder(); sqlBuilder.append(" pfv").append(n).append(".value IS NULL"); - - sqlCountBuilder = new StringBuilder(); - sqlCountBuilder.append(" fv.value IS NULL"); } break; @@ -541,11 +520,9 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { if (submissionListColumn.getExactMatch()) { // perform exact match sqlBuilder.append("pfv").append(n).append(".value = '").append(filterString).append("'"); - sqlCountBuilder.append("fv.value = '").append(filterString).append("'"); } else { // perform like when input from text field sqlBuilder.append("LOWER(pfv").append(n).append(".value) LIKE '%").append(escapeString(filterString)).append("%'"); - sqlCountBuilder.append("LOWER(fv.value) LIKE '%").append(escapeString(filterString)).append("%'"); } break; @@ -553,12 +530,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { if (sqlBuilder.length() > 0) { sqlWhereBuilderList.add(sqlBuilder); - - if (!sqlCountWherePredicate.containsKey(predicateId)) { - sqlCountWherePredicate.put(predicateId, new StringBuilder()); - } - - sqlCountWherePredicate.get(predicateId).append(" (").append(sqlCountBuilder).append(") OR"); } } @@ -589,7 +560,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder = new StringBuilder(); sqlBuilder.append("s").append(".id = ").append(filterString); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "id").add(sqlBuilder); } break; @@ -599,7 +569,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\nLEFT JOIN submission_status ss ON ss.id=s.submission_status_id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "ss.name"); @@ -616,7 +585,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "submissionStatus.name").add(sqlBuilder); } // all column search filter @@ -635,7 +603,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\nLEFT JOIN organization o ON o.id=s.organization_id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); } if (submissionListColumn.getSortOrder() > 0) { @@ -654,7 +621,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "organization.name").add(sqlBuilder); } // all column search filter @@ -675,7 +641,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder.append("\nLEFT JOIN organization_category oc ON oc.id=o.category_id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "oc.name"); @@ -692,7 +657,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "organization.category.name").add(sqlBuilder); } // all column search filter @@ -709,7 +673,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\nLEFT JOIN weaver_users a ON a.id=s.assignee_id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "a.email"); @@ -727,7 +690,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "assignee.email").add(sqlBuilder); } // all column search filter @@ -749,7 +711,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\n ON embs.submission_id=s.id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "embs.name"); @@ -767,7 +728,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "embargoTypes.name").add(sqlBuilder); } // all column search filter @@ -790,7 +750,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\n ON action_logs_id = s.submission_status_id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); // TODO: finish sqlWheresBuilder. @@ -821,7 +780,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("submission_date", filterString); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "submissionDate").add(sqlBuilder); } break; @@ -834,7 +792,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("approve_application_date", filterString); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "approveApplicationDate").add(sqlBuilder); } break; @@ -847,7 +804,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("approve_advisor_date", filterString); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "approveAdvisorDate").add(sqlBuilder); } break; @@ -860,7 +816,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("approve_embargo_date", filterString); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "approveEmbargoDate").add(sqlBuilder); } break; @@ -874,13 +829,11 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\n ON scavcavcad.submission_id = s.id"); sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = new StringBuilder(); sqlBuilder.append("scavcavcad.value = true AND scavcavcad.label = '" + filterString + "'"); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "customActionValues").add(sqlBuilder); } break; @@ -894,7 +847,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder = new StringBuilder(); sqlBuilder.append("LOWER(s").append(".depositurl) LIKE '%").append(escapeString(filterString)).append("%'"); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "depositurl").add(sqlBuilder); } // all column search filter @@ -915,7 +867,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder = new StringBuilder(); sqlBuilder.append("LOWER(s").append(".reviewer_notes) LIKE '%").append(escapeString(filterString)).append("%'"); sqlWhereBuilderList.add(sqlBuilder); - getFromBuildersMap(sqlCountWhereFilterBuilders, "reviewer_notes").add(sqlBuilder); } // all column search filter @@ -942,7 +893,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { }); sqlSelectBuilder.setLength(sqlSelectBuilder.length() - 2); sqlSelectBuilder.append(" FROM submission s"); - sqlCountSelectBuilder.insert(0, "SELECT COUNT(DISTINCT s.id) FROM submission s"); // if ordering, complete order by clause and strip the tailing comma if (sqlOrderBysBuilder.length() > 0) { @@ -989,54 +939,8 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } } - if (sqlCountWherePredicate.size() > 0 || sqlCountWhereFilterBuilders.size() > 0 || sqlWheresExcludeBuilder.length() > 0) { - - // Conditions are AND across different predicates but are OR within the same predicate. - if (sqlCountWherePredicate.size() > 0) { - sqlCountSelectBuilder.append("\nLEFT JOIN submission_field_values sfv ON s.id = sfv.submission_id"); - sqlCountSelectBuilder.append("\nINNER JOIN field_value fv ON sfv.field_values_id = fv.id"); - sqlCountSelectBuilder.append("\nWHERE"); - - sqlCountWherePredicate.forEach((id, filter) -> { - if (filter.length() > 0) { - // Remove the last " OR". - filter.setLength(filter.length() - 3); - - sqlCountSelectBuilder - .append("\n(fv.field_predicate_id = ").append(id) - .append(" AND (").append(filter).append(")) AND"); - } - }); - } else { - sqlCountSelectBuilder.append("\nWHERE"); - } - - // Conditions are AND across different filters and OR within the same filter. - if (sqlCountWhereFilterBuilders.size() > 0) { - sqlCountWhereFilterBuilders.forEach((key, list) -> { - sqlCountSelectBuilder.append(" ("); - - list.forEach(filter -> { - sqlCountSelectBuilder.append(" (").append(filter).append(") OR"); - }); - - // Remove the last " OR". - sqlCountSelectBuilder.setLength(sqlCountSelectBuilder.length() - 3); - - sqlCountSelectBuilder.append(")\n AND"); - }); - } - - if (sqlWheresExcludeBuilder.length() > 0) { - sqlCountSelectBuilder.append("\n(").append(sqlWheresExcludeBuilder).append(")"); - } else { - // remove last " AND" - sqlCountSelectBuilder.setLength(sqlCountSelectBuilder.length() - 4); - } - } - String sqlQuery = sqlSelectBuilder.toString() + sqlJoinsBuilder.toString() + sqlBuilder.toString(); - String sqlCountQuery = sqlCountSelectBuilder.toString(); + String sqlCountQuery = "SELECT COUNT(DISTINCT s.id) FROM submission s" + sqlJoinsBuilder.toString() + sqlBuilder.toString(); if (pageable != null) { // determine the offset and limit of the query @@ -1130,22 +1034,6 @@ private StringBuilder buildSubmissionDateFieldString(String column, String filte .append("' AS DATE)"); } - /** - * Get the builders list array for some key, initializing that key if not found. - * - * @param map The map of builders to select from. - * @param key The identifier. - * - * @return An array of the builders for the given key. - */ - private ArrayList getFromBuildersMap(Map> map, String key) { - if (!map.containsKey(key)) { - map.put(key, new ArrayList()); - } - - return map.get(key); - } - /** * Add SQL escape protection for string, defaulting to forcing lower case. * From 56712666a805e702d9e52ccd66cae0d1a956505f Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Wed, 8 May 2024 15:07:12 -0500 Subject: [PATCH 2/3] Revert "Issue 1931: Pagination not working properly for combined filters list." This reverts commit 5ce9f384c23539d89876a408c6ff43878cda0364. The plan is to go with a smarter design that conditionally uses the aggressive query optimization. --- .../model/repo/impl/SubmissionRepoImpl.java | 114 +++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java index 26b36e5e6..79fb337c0 100644 --- a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java +++ b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java @@ -429,12 +429,16 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { }); StringBuilder sqlSelectBuilder = new StringBuilder("SELECT DISTINCT "); + StringBuilder sqlCountSelectBuilder = new StringBuilder(); Map> sqlColumnsBuilders = new HashMap<>(); + Map> sqlCountWhereFilterBuilders = new HashMap<>(); + Map sqlCountWherePredicate = new HashMap<>(); List sqlAliasBuilders = new ArrayList<>(); StringBuilder sqlJoinsBuilder = new StringBuilder(); StringBuilder sqlBuilder; + StringBuilder sqlCountBuilder; StringBuilder sqlWheresExcludeBuilder = new StringBuilder(); StringBuilder sqlOrderBysBuilder = new StringBuilder(); @@ -482,11 +486,13 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = new StringBuilder(); + sqlCountBuilder = new StringBuilder(); switch (submissionListColumn.getInputType().getName()) { case "INPUT_DEGREEDATE": // Column's values are of type 'MMMM yyyy' (in SQL date format would be 'Month YYYY'). sqlBuilder.append("LOWER(pfv").append(n).append(".value) = LOWER('").append(filterString).append("')"); + sqlCountBuilder.append("LOWER(fv.value) = LOWER('").append(filterString).append("')"); break; case "INPUT_DATE": // Column's values are of type 'yyyy-mm-dd' as required by the SQL standard to represent a date without time. @@ -498,20 +504,35 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append(".value AS DATE) BETWEEN CAST('").append(dates[0]) .append("' AS DATE) AND CAST('").append(dates[1]) .append("' AS DATE)"); + sqlCountBuilder + .append("CAST(fv.value AS DATE) BETWEEN CAST('").append(dates[0]) + .append("' AS DATE) AND CAST('").append(dates[1]) + .append("' AS DATE)"); } else { // Date Match sqlBuilder.append("pfv").append(n).append(".value = '").append(filterString).append("'"); + sqlCountBuilder.append("fv.value = '").append(filterString).append("'"); } break; case "INPUT_CHECKBOX": sqlBuilder.append("pfv").append(n).append(".value = '").append(filterString).append("'"); + sqlCountBuilder.append("fv.value = '").append(filterString).append("'"); // Column's values are a boolean if (!Boolean.valueOf(filterString)) { sqlWhereBuilderList.add(sqlBuilder); + if (!sqlCountWherePredicate.containsKey(predicateId)) { + sqlCountWherePredicate.put(predicateId, new StringBuilder()); + } + + sqlCountWherePredicate.get(predicateId).append(" (").append(sqlCountBuilder).append(") OR"); + sqlBuilder = new StringBuilder(); sqlBuilder.append(" pfv").append(n).append(".value IS NULL"); + + sqlCountBuilder = new StringBuilder(); + sqlCountBuilder.append(" fv.value IS NULL"); } break; @@ -520,9 +541,11 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { if (submissionListColumn.getExactMatch()) { // perform exact match sqlBuilder.append("pfv").append(n).append(".value = '").append(filterString).append("'"); + sqlCountBuilder.append("fv.value = '").append(filterString).append("'"); } else { // perform like when input from text field sqlBuilder.append("LOWER(pfv").append(n).append(".value) LIKE '%").append(escapeString(filterString)).append("%'"); + sqlCountBuilder.append("LOWER(fv.value) LIKE '%").append(escapeString(filterString)).append("%'"); } break; @@ -530,6 +553,12 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { if (sqlBuilder.length() > 0) { sqlWhereBuilderList.add(sqlBuilder); + + if (!sqlCountWherePredicate.containsKey(predicateId)) { + sqlCountWherePredicate.put(predicateId, new StringBuilder()); + } + + sqlCountWherePredicate.get(predicateId).append(" (").append(sqlCountBuilder).append(") OR"); } } @@ -560,6 +589,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder = new StringBuilder(); sqlBuilder.append("s").append(".id = ").append(filterString); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "id").add(sqlBuilder); } break; @@ -569,6 +599,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\nLEFT JOIN submission_status ss ON ss.id=s.submission_status_id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "ss.name"); @@ -585,6 +616,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "submissionStatus.name").add(sqlBuilder); } // all column search filter @@ -603,6 +635,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\nLEFT JOIN organization o ON o.id=s.organization_id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); } if (submissionListColumn.getSortOrder() > 0) { @@ -621,6 +654,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "organization.name").add(sqlBuilder); } // all column search filter @@ -641,6 +675,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder.append("\nLEFT JOIN organization_category oc ON oc.id=o.category_id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "oc.name"); @@ -657,6 +692,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "organization.category.name").add(sqlBuilder); } // all column search filter @@ -673,6 +709,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\nLEFT JOIN weaver_users a ON a.id=s.assignee_id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "a.email"); @@ -690,6 +727,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "assignee.email").add(sqlBuilder); } // all column search filter @@ -711,6 +749,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\n ON embs.submission_id=s.id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); if (submissionListColumn.getSortOrder() > 0) { setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "embs.name"); @@ -728,6 +767,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "embargoTypes.name").add(sqlBuilder); } // all column search filter @@ -750,6 +790,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\n ON action_logs_id = s.submission_status_id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); // TODO: finish sqlWheresBuilder. @@ -780,6 +821,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("submission_date", filterString); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "submissionDate").add(sqlBuilder); } break; @@ -792,6 +834,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("approve_application_date", filterString); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "approveApplicationDate").add(sqlBuilder); } break; @@ -804,6 +847,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("approve_advisor_date", filterString); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "approveAdvisorDate").add(sqlBuilder); } break; @@ -816,6 +860,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = buildSubmissionDateFieldString("approve_embargo_date", filterString); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "approveEmbargoDate").add(sqlBuilder); } break; @@ -829,11 +874,13 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { .append("\n ON scavcavcad.submission_id = s.id"); sqlJoinsBuilder.append(sqlBuilder); + sqlCountSelectBuilder.append(sqlBuilder); for (String filterString : submissionListColumn.getFilters()) { sqlBuilder = new StringBuilder(); sqlBuilder.append("scavcavcad.value = true AND scavcavcad.label = '" + filterString + "'"); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "customActionValues").add(sqlBuilder); } break; @@ -847,6 +894,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder = new StringBuilder(); sqlBuilder.append("LOWER(s").append(".depositurl) LIKE '%").append(escapeString(filterString)).append("%'"); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "depositurl").add(sqlBuilder); } // all column search filter @@ -867,6 +915,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlBuilder = new StringBuilder(); sqlBuilder.append("LOWER(s").append(".reviewer_notes) LIKE '%").append(escapeString(filterString)).append("%'"); sqlWhereBuilderList.add(sqlBuilder); + getFromBuildersMap(sqlCountWhereFilterBuilders, "reviewer_notes").add(sqlBuilder); } // all column search filter @@ -893,6 +942,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { }); sqlSelectBuilder.setLength(sqlSelectBuilder.length() - 2); sqlSelectBuilder.append(" FROM submission s"); + sqlCountSelectBuilder.insert(0, "SELECT COUNT(DISTINCT s.id) FROM submission s"); // if ordering, complete order by clause and strip the tailing comma if (sqlOrderBysBuilder.length() > 0) { @@ -939,8 +989,54 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } } + if (sqlCountWherePredicate.size() > 0 || sqlCountWhereFilterBuilders.size() > 0 || sqlWheresExcludeBuilder.length() > 0) { + + // Conditions are AND across different predicates but are OR within the same predicate. + if (sqlCountWherePredicate.size() > 0) { + sqlCountSelectBuilder.append("\nLEFT JOIN submission_field_values sfv ON s.id = sfv.submission_id"); + sqlCountSelectBuilder.append("\nINNER JOIN field_value fv ON sfv.field_values_id = fv.id"); + sqlCountSelectBuilder.append("\nWHERE"); + + sqlCountWherePredicate.forEach((id, filter) -> { + if (filter.length() > 0) { + // Remove the last " OR". + filter.setLength(filter.length() - 3); + + sqlCountSelectBuilder + .append("\n(fv.field_predicate_id = ").append(id) + .append(" AND (").append(filter).append(")) AND"); + } + }); + } else { + sqlCountSelectBuilder.append("\nWHERE"); + } + + // Conditions are AND across different filters and OR within the same filter. + if (sqlCountWhereFilterBuilders.size() > 0) { + sqlCountWhereFilterBuilders.forEach((key, list) -> { + sqlCountSelectBuilder.append(" ("); + + list.forEach(filter -> { + sqlCountSelectBuilder.append(" (").append(filter).append(") OR"); + }); + + // Remove the last " OR". + sqlCountSelectBuilder.setLength(sqlCountSelectBuilder.length() - 3); + + sqlCountSelectBuilder.append(")\n AND"); + }); + } + + if (sqlWheresExcludeBuilder.length() > 0) { + sqlCountSelectBuilder.append("\n(").append(sqlWheresExcludeBuilder).append(")"); + } else { + // remove last " AND" + sqlCountSelectBuilder.setLength(sqlCountSelectBuilder.length() - 4); + } + } + String sqlQuery = sqlSelectBuilder.toString() + sqlJoinsBuilder.toString() + sqlBuilder.toString(); - String sqlCountQuery = "SELECT COUNT(DISTINCT s.id) FROM submission s" + sqlJoinsBuilder.toString() + sqlBuilder.toString(); + String sqlCountQuery = sqlCountSelectBuilder.toString(); if (pageable != null) { // determine the offset and limit of the query @@ -1034,6 +1130,22 @@ private StringBuilder buildSubmissionDateFieldString(String column, String filte .append("' AS DATE)"); } + /** + * Get the builders list array for some key, initializing that key if not found. + * + * @param map The map of builders to select from. + * @param key The identifier. + * + * @return An array of the builders for the given key. + */ + private ArrayList getFromBuildersMap(Map> map, String key) { + if (!map.containsKey(key)) { + map.put(key, new ArrayList()); + } + + return map.get(key); + } + /** * Add SQL escape protection for string, defaulting to forcing lower case. * From 61f002d9db11e4e2f6bed8d1ebc8e6c4b5468661 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Wed, 8 May 2024 15:08:05 -0500 Subject: [PATCH 3/3] Issue 1931: Pagination not working properly for combined filters list. This is a regression from the aggressive performance optimizations such as commit b2c538c5c9c7d99a18c9f8c2c47b8cc98c01a260. The case that is not correctly handled is where there are multiple separate field values being selected and filtered upon. The solution here is different from the previous solution in commit 5ce9f384c23539d89876a408c6ff43878cda0364. This approach preserves the aggressive optimization but when there are more than one filter values being filter it then disables the filter. This allows for using the aggressive optimization whenever possible. This uses the basic non-optimized select query (with a count) when there are two or more field values. --- .../vireo/model/repo/impl/SubmissionRepoImpl.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java index 79fb337c0..29a6897b6 100644 --- a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java +++ b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java @@ -449,6 +449,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { sqlAliasBuilders.add("s.id"); int n = 0; + int totalFieldValueConditions = 0; for (SubmissionListColumn submissionListColumn : allSubmissionListColumns) { @@ -552,6 +553,7 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } if (sqlBuilder.length() > 0) { + totalFieldValueConditions++; sqlWhereBuilderList.add(sqlBuilder); if (!sqlCountWherePredicate.containsKey(predicateId)) { @@ -942,7 +944,6 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { }); sqlSelectBuilder.setLength(sqlSelectBuilder.length() - 2); sqlSelectBuilder.append(" FROM submission s"); - sqlCountSelectBuilder.insert(0, "SELECT COUNT(DISTINCT s.id) FROM submission s"); // if ordering, complete order by clause and strip the tailing comma if (sqlOrderBysBuilder.length() > 0) { @@ -1036,7 +1037,14 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { } String sqlQuery = sqlSelectBuilder.toString() + sqlJoinsBuilder.toString() + sqlBuilder.toString(); - String sqlCountQuery = sqlCountSelectBuilder.toString(); + String sqlCountQuery = "SELECT COUNT(DISTINCT s.id) FROM submission s"; + + // Use count query optimization only when there are fewer than 2 field values in the where clause. + if (totalFieldValueConditions > 1) { + sqlCountQuery += sqlJoinsBuilder.toString() + sqlBuilder.toString(); + } else { + sqlCountQuery += sqlCountSelectBuilder.toString(); + } if (pageable != null) { // determine the offset and limit of the query