Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove junctions from query engine API [DHIS2-18041] #20008

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Feb 20, 2025

Summary

Fixes special filters that would require junctions by moving their composition into the query engine.

While this refactoring removes nested junctions from the query engine API this does not remove a web API feature. Nested junctions were only used in a few special programmatic cases to build more sophisticated filters. Within the Query all filters are now just one Restriction. Sophisticated ones are expanded first within the engine making it unnecessary to model junctions on the Query or Restriction level.

To distinguish Restrictions that are simple from those that need expanding the isVirtual() method got added encoding the few conditions where this is needed.

Simplifications

  • removed Criteria, Criterion, Disjunction and Conjunction as all filters are now simple Restrictions
  • aliases are no longer in Query but extracted from the Restriction in the JPA query engine when needed
  • Typed got replaced by simply using List.of
  • Operator.collectionArgs was replaced with using Operator.args instead (there never was a need for another field as only in/not-in operator would use it and they always would only have one list of args that could use args as all other operators)
  • QueryPlanner.planQuery with persistedOnly got removed (this was never necessary as Query instances passed to a QueryEngine should always be already planned as they should come from within the QueryService and never be passed to the engine directly)

Corrections

  • DefaultJpaQueryParser got renamed to DefaultQueryParser (wasn't specific to JPA in any way)
  • QueryEngine generic got removed (it was wrongly made generic when it was not)
  • the generic on Operator got changed from <T extends Comparable<? super T>> to <T extends Comparable<T>> (was too unspecific which forced some casts that now can be avoided)

Cleanup

  • QueryPath got moved into the schema module and is not computed by the SchemaService (as it really as an extension on Schema and Property that is only used for Query)
  • QueryService.query with ResultTransformer got removed (was unused)
  • Operator.getHibernateCriterion got removed (was unused)

@jbee jbee self-assigned this Feb 20, 2025
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
9 New issues
9 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@jbee jbee changed the title refactor: remove query engine nested junction support [DHIS2-18041] refactor: remove query engine nested junctions [DHIS2-18041] Feb 20, 2025
disjunctions.add(disjunction);
}
return disjunctions;
private Restriction getDisjunctionsFromCustomMentions(List<String> mentions, Schema schema) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'schema' is never used.

Copilot Autofix AI 3 days ago

To fix the problem, we need to remove the unused parameter Schema schema from the method getDisjunctionsFromCustomMentions. This involves:

  1. Removing the parameter from the method signature.
  2. Updating any calls to this method to remove the corresponding argument.

This change will simplify the method and its usage without altering its functionality.

Suggested changeset 1
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/DefaultQueryParser.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/DefaultQueryParser.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/DefaultQueryParser.java
--- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/DefaultQueryParser.java
+++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/DefaultQueryParser.java
@@ -161,3 +161,3 @@
 
-  private Restriction getDisjunctionsFromCustomMentions(List<String> mentions, Schema schema) {
+  private Restriction getDisjunctionsFromCustomMentions(List<String> mentions) {
     List<String> items = new ArrayList<>();
EOF
@@ -161,3 +161,3 @@

private Restriction getDisjunctionsFromCustomMentions(List<String> mentions, Schema schema) {
private Restriction getDisjunctionsFromCustomMentions(List<String> mentions) {
List<String> items = new ArrayList<>();
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@ToString(onlyExplicitlyIncluded = true)
public class Query {

@ToString.Include private final List<Restriction> criterions = new ArrayList<>();

Check notice

Code scanning / CodeQL

Exposing internal representation Note

getCriterions exposes the internal representation stored in field criterions. The value may be modified
after this call to getCriterions
.
@jbee jbee changed the title refactor: remove query engine nested junctions [DHIS2-18041] refactor: remove junctions query engine API [DHIS2-18041] Feb 20, 2025
@jbee jbee changed the title refactor: remove junctions query engine API [DHIS2-18041] refactor: remove junctions from query engine API [DHIS2-18041] Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant