feat(ontology): refactor to 'lazy' ontology input and filter so heavy lifting is moved to backend and we don't need to download complete ontologies #4584

mswertz · 2024-12-29T22:06:10Z

What are the main changes you did

we now get very big queries when you select root nodes in large ontologies, because all children are included as seperate filters. Makes all slow. Instead this PR aims to do the heavy lifting server side. Also will allow data to be smaller, and excel/csv uploads easier, when users can simply include the terms they mean without the sub terms.

N.B. blocks on some of the changes from #4539

How to test

see junit tests
see changes to the javascript => see that URLs are smaller

Checklist

updated docs in case of new feature
added/updated tests
created 'match_including_children'
created 'match_including_parents'
created 'contains_any' that should be faster that current nested 'equal'
created 'contains_all' that should be faster than complex 'and' in directory
create a 'is_null' and 'is_not_null' filter so we can get root elements (parent == null)
use 'match_including_children' in ontology filters instead of sending all children elements

Do later

lazy load the ontology contents so subtree data is only loaded when user clicks
enable search on the lazy loaded ontology (should return parent paths for search matches)

…uch faster

…ontology_subtree_filter

mswertz · 2025-01-28T17:36:31Z

backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlQuery.java

@@ -1,7 +1,6 @@
 package org.molgenis.emx2.sql;


restructured this file so more changes than needed but I hope will make this huge file more easy to understand

mswertz · 2025-01-28T17:38:45Z

backend/molgenis-emx2/src/test/java/org/molgenis/emx2/TestTypeUtils.java

@@ -99,30 +99,29 @@ void testToJsonb() throws JsonProcessingException {
    String trailingData = "{\"key\":\"value\"}trailing";
    int invalidJavaType = 1;

-    assertAll(
-        // valid: object


refactor so individual errors are shown

mswertz · 2025-01-28T17:39:07Z

docs/molgenis/dev_graphql.md

@@ -638,6 +638,7 @@ the following function are available:
 - textSearch(value)
 - between(value)
 - notBetween(value)
+- _match_any_in_subtree(name) - use this to filter in ontology columns matching also when overlap exists in children of 'name' term


todo update docs with all these changes

mswertz · 2025-01-29T11:27:56Z

use case not coverered is when a data entry says 'select all cancers' and then I search for 'a specific cancer'.

sonarqubecloud · 2025-01-30T08:39:44Z

Quality Gate passed

Issues
27 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

harmbrugge

I think it would be useful to go trough the SqlQuery and the actual sql function together, because it's hard for me to fully understand what is going on

harmbrugge · 2025-02-04T08:29:23Z

.../molgenis-emx2-graphql/src/main/java/org/molgenis/emx2/graphql/GraphqlTableFieldFactory.java

+          .name("MolgenisIsNotNullEnum")
+          .value(IsNullOrNotNull.NULL.name(), IsNullOrNotNull.NULL)
+          .value(IsNullOrNotNull.NOT_NULL.name(), IsNullOrNotNull.NOT_NULL)
+          .build();
  private static GraphQLObjectType fileDownload =


Just for my knowledge: Does an enum give you a certain behaviour as a graphql type?

harmbrugge · 2025-02-04T08:41:27Z

.../molgenis-emx2-graphql/src/main/java/org/molgenis/emx2/graphql/GraphqlTableFieldFactory.java

+          filterBuilder.field(
+              GraphQLInputObjectField.newInputObjectField()
+                  .name(FILTER_IS)
+                  .type(isNullOrNotNullEnum)


Ah here we go, the enum gives you fixed options

Wouldn't it make more sense to make this a boolean operator _is_null true false?

harmbrugge · 2025-02-04T09:04:47Z

.../molgenis-emx2-graphql/src/main/java/org/molgenis/emx2/graphql/GraphqlTableFieldFactory.java

+          value.remove(FILTER_CONTAINS_ALL);
+        }
+
+        if (value.size() == 0) continue;


This function is getting a bit chunky

harmbrugge · 2025-02-04T09:06:18Z

...d/molgenis-emx2-graphql/src/test/java/org/molgenis/emx2/graphql/TestGraphqlSchemaFields.java

+    String result =
+        execute("{Pet(filter:{tags:{_match_any_including_children:\"colors\"}}){name}}").toString();
+    assertTrue(result.contains("tom"));
+    assertFalse(result.contains("pooky")); // poor pooky has no color


harmbrugge · 2025-02-04T09:23:22Z

backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlQuery.java

+        column,
+        DSL.select(
+            field(
+                "\"MOLGENIS\".get_terms_including_children({0},{1},{2})",


This is nice, you delegated it to the db

harmbrugge · 2025-02-04T09:25:39Z

backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlQuery.java

+              .map(
+                  value ->
+                      condition(
+                          "0 < ( SELECT COUNT(*) FROM unnest({1}) AS v WHERE to_tsquery({0}) @@ to_tsvector(v)",


I do not fully comprehend this

chinook25

Haven't gone through it all yet, but here are some comments of what I've found thus far.

chinook25 · 2025-01-30T09:08:31Z

apps/molgenis-components/src/components/forms/InputOntology.vue

@@ -286,6 +287,7 @@ export default {
      this.key++;
    },
    deselect(item: string) {
+      console.log("haat " + item);


don't forget to remove

chinook25 · 2025-01-30T09:10:29Z

apps/molgenis-components/src/components/tables/TableExplorer.vue

          filter[col.id] = { equals: conditions };
+        } else if (col.columnType.startsWith("ONTOLOGY")) {
+          filter[col.id] = {
+            match_including_children: conditions.map((term) => term.name),


TS error, term is supposed to be a string, so term.name is not possible

chinook25 · 2025-01-30T09:51:03Z

.../molgenis-emx2-graphql/src/main/java/org/molgenis/emx2/graphql/GraphqlTableFieldFactory.java

@@ -577,6 +637,51 @@ public static FilterBean[] convertMapToFilterArray(
                  + " unknown in table "
                  + table.getTableName());
        Column c = optional.get();
+        Map value = (Map) entry.getValue();


This function is quite rough to read due to it's logical complexity. I suggest moving the content of the outer ifs (the ones starting on line 595) to functions to make it more clear what is going on.

chinook25 · 2025-01-30T09:56:33Z

...d/molgenis-emx2-graphql/src/test/java/org/molgenis/emx2/graphql/TestGraphqlSchemaFields.java

+  }
+
+  @Test
+  void testNullAndNotNullt() throws IOException {


chinook25 · 2025-01-30T10:53:23Z

backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlQuery.java

+              .as(name(convertToPascalCase(select.getColumn()))));
+    }
+
+    // asemble final query


chinook25 · 2025-01-30T10:59:13Z

backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlQuery.java

+    SelectConnectByStep<org.jooq.Record> from =
+        fromQuery(table, List.of(asterisk()), column, tableAlias, subAlias, filters, searchTerms);
+    from = applyLimitOffsetOrderBy(table, select, from, subAlias);


Having a far immediately be overwritten by a modified version feels a bit weird. Suggest first assigning an intermediate var and have from not be overwritten.

mswertz added 5 commits December 27, 2024 20:18

first commit

2182f24

updated molgenis-components

0f47494

make ontology inputs faster

412c777

update

474218a

integrate search into tree fixing some bugs as we go, and making it m…

ce76e0f

…uch faster

mswertz marked this pull request as draft December 29, 2024 22:06

mswertz changed the title ~~feat: new backend operator to do ontology filter expansion to children server side~~ feat: new backend operator '_match_any_in_subtree' to do ontology filter expansion to children server side Dec 29, 2024

pull fixes from subtree PR

3e5f5f3

mswertz mentioned this pull request Dec 30, 2024

feat: fix ontology tree ui performance and enabled large disease ontologies filter on catalogue #4539

Merged

4 tasks

pull changes

2662fba

mswertz changed the title ~~feat: new backend operator '_match_any_in_subtree' to do ontology filter expansion to children server side~~ feat: for ontologies, new backend operator '_match_any_in_subtree' to do ontology filter expansion to children server side Jan 12, 2025

mswertz added 4 commits January 12, 2025 20:41

partial sync

1458e5b

close to merge

47bf393

merge master

7e6e5be

add match_including_children

a4aec60

mswertz added 3 commits January 14, 2025 17:12

wip

b725671

Merge branch 'master' of github.com:molgenis/molgenis-emx2 into feat/…

1ec0083

…ontology_subtree_filter

switch to server side expansion

a87e163

feat: add null and not null filter

7e5a95b

chinook25 assigned mswertz Jan 16, 2025

mswertz added 6 commits January 20, 2025 00:19

refactor

2e70a57

Merge branch 'master' of github.com:molgenis/molgenis-emx2 into feat/…

04e8d7e

…ontology_subtree_filter

remove unused code

608f59b

fix merge error

07e5181

fix merge error

18ff7e3

Merge branch 'master' of github.com:molgenis/molgenis-emx2 into feat/…

801a274

…ontology_subtree_filter

mswertz added 11 commits January 25, 2025 21:58

refactor

d859290

fix

09321df

fix test

f13c9ae

another fix

2c4e457

fix

a326eb3

remove unused code

69ba407

major refactor

950a5e1

disable contains_all for now

bf9eaeb

disable wip on contains_all for now

b108c9c

wip

58e7212

Merge branch 'master' of github.com:molgenis/molgenis-emx2 into feat/…

9215fbe

…ontology_subtree_filter

mswertz commented Jan 28, 2025

View reviewed changes

partial fix contains_all

273c950

Merge branch 'master' into feat/ontology_subtree_filter

ea16484

mswertz requested a review from chinook25 January 29, 2025 22:51

mswertz added 3 commits January 30, 2025 00:18

refactor to have _ prefix on operators

02ab45b

add match_path

7c19665

update docs

92a2749

mswertz marked this pull request as ready for review January 30, 2025 08:50

harmbrugge reviewed Feb 4, 2025

View reviewed changes

chinook25 reviewed Feb 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ontology): refactor to 'lazy' ontology input and filter so heavy lifting is moved to backend and we don't need to download complete ontologies #4584

feat(ontology): refactor to 'lazy' ontology input and filter so heavy lifting is moved to backend and we don't need to download complete ontologies #4584

mswertz commented Dec 29, 2024 •

edited

Loading

mswertz Jan 28, 2025

mswertz Jan 28, 2025

mswertz Jan 28, 2025

mswertz commented Jan 29, 2025

sonarqubecloud bot commented Jan 30, 2025

harmbrugge left a comment

harmbrugge Feb 4, 2025

harmbrugge Feb 4, 2025

harmbrugge Feb 4, 2025

harmbrugge Feb 4, 2025

harmbrugge Feb 4, 2025

harmbrugge Feb 4, 2025

harmbrugge Feb 4, 2025

chinook25 left a comment

chinook25 Jan 30, 2025

chinook25 Jan 30, 2025

chinook25 Jan 30, 2025

chinook25 Jan 30, 2025

chinook25 Jan 30, 2025

chinook25 Jan 30, 2025

feat(ontology): refactor to 'lazy' ontology input and filter so heavy lifting is moved to backend and we don't need to download complete ontologies #4584

Are you sure you want to change the base?

feat(ontology): refactor to 'lazy' ontology input and filter so heavy lifting is moved to backend and we don't need to download complete ontologies #4584

Conversation

mswertz commented Dec 29, 2024 • edited Loading

What are the main changes you did

How to test

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mswertz commented Jan 29, 2025

sonarqubecloud bot commented Jan 30, 2025

Quality Gate passed

harmbrugge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinook25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mswertz commented Dec 29, 2024 •

edited

Loading