feat: Improve TEI Performance [DHIS2-14884] #14039

gnespolino · 2023-05-25T14:14:14Z

This PR will improve TEI performance by producing a different and optimized query.
It is based on this doc:
https://docs.google.com/document/d/1tcqDoOvKLW5W3t7HNApxbV9cEcMTq6iJIwoG2tZOsD0/edit?usp=sharing

maikelarabori

I left a few comments Giuseppe.
It looks good in general, but I'm a bit concerned about the performance when the user hits the "Download" button where we don't have any pagination in place...

Also, please don't forget to add a few good e2e tests so we can have a good coverage on it.

Thanks,
Maikel

maikelarabori · 2023-06-07T09:41:23Z

...es/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/SqlQueryExecutor.java


-        SqlRowSet rowSet = namedParameterJdbcTemplate.queryForRowSet( query.getStatement(),
-            new MapSqlParameterSource().addValues( query.getParams() ) );
+        SqlQuery forSelect = queryCreator.createForSelect();


I believe is better to keep the SqlQuery as parameter, as this can be created from outside.
It's sounds strange that "find" or "count" receives a "creator component". Receiving a sql query sounds more natural and more consistent.

but then I should pass the SqlQueryExecutor 2 different SQLQuery: one for find, one for count

...ervices/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/query/Field.java

...va/org/hisp/dhis/analytics/common/query/jsonextractor/AggregatedJsonExtractingSqlRowSet.java

...cs/src/main/java/org/hisp/dhis/analytics/common/query/jsonextractor/EnrollmentExtractor.java

...va/org/hisp/dhis/analytics/common/query/jsonextractor/AggregatedJsonExtractingSqlRowSet.java

...cs/src/main/java/org/hisp/dhis/analytics/common/query/jsonextractor/DelegatingSqlRowSet.java

maikelarabori

I left a few comments Giuseppe.
It looks good in general, but I'm a bit concerned about the performance when the user hits the "Download" button where we don't have any pagination in place...

Also, please don't forget to add a few good e2e tests so we can have a nice coverage on it.

Thanks,
Maikel

codecov · 2023-07-18T12:10:47Z

Codecov Report

Attention: Patch coverage is 15.46961% with 153 lines in your changes are missing coverage. Please review.

Project coverage is 67.11%. Comparing base (672f399) to head (03174cd).

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14039      +/-   ##
============================================
- Coverage     67.18%   67.11%   -0.07%     
- Complexity    32155    32159       +4     
============================================
  Files          3556     3560       +4     
  Lines        131776   131935     +159     
  Branches      15329    15340      +11     
============================================
+ Hits          88528    88546      +18     
- Misses        36063    36196     +133     
- Partials       7185     7193       +8

Flag	Coverage Δ
integration	`50.14% <1.10%> (-0.07%)`	⬇️
integration-h2	`34.08% <1.10%> (-0.04%)`	⬇️
unit	`30.65% <15.46%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...org/hisp/dhis/analytics/common/SqlQueryResult.java	`50.00% <ø> (ø)`
...va/org/hisp/dhis/analytics/common/query/Field.java	`85.71% <100.00%> (+1.50%)`	⬆️
.../context/querybuilder/DataElementQueryBuilder.java	`92.85% <100.00%> (+0.17%)`	⬆️
...uery/context/querybuilder/OrgUnitQueryBuilder.java	`76.47% <100.00%> (+0.71%)`	⬆️
...ei/query/context/querybuilder/TeiQueryBuilder.java	`75.86% <100.00%> (+6.29%)`	⬆️
...tics/tei/query/context/sql/RenderableSqlQuery.java	`72.22% <100.00%> (+1.63%)`	⬆️
...alytics/tei/query/context/sql/SqlQueryCreator.java	`33.33% <ø> (ø)`
...pi/src/main/java/org/hisp/dhis/util/DateUtils.java	`83.00% <66.66%> (-0.25%)`	⬇️
...s/common/params/dimension/DimensionIdentifier.java	`75.00% <0.00%> (-2.42%)`	⬇️
...lytics/common/params/dimension/DimensionParam.java	`69.81% <0.00%> (-0.67%)`	⬇️
... and 8 more

... and 6 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 672f399...03174cd. Read the comment docs.

sonarcloud · 2023-07-20T07:42:13Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

0.0% Coverage
0.0% Duplication

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

sonarcloud · 2023-08-21T09:23:07Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

0.0% Coverage
0.0% Duplication

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

…N-EXTRACTOR # Conflicts: # dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/TeiAnalyticsQueryService.java

…N-EXTRACTOR

...ics/src/main/java/org/hisp/dhis/analytics/common/query/jsonextractor/SqlRowSetDelegator.java

…N-EXTRACTOR

…4884]

…into DHIS2-14884_POC-JSON-EXTRACTOR

...src/test/java/org/hisp/dhis/analytics/common/query/jsonextractor/JsonExtractorUtilsTest.java

+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class JsonExtractorUtilsTest {


maikelarabori · 2024-06-12T10:02:49Z

...ics/src/main/java/org/hisp/dhis/analytics/common/query/jsonextractor/JsonExtractorUtils.java

+  private static final Pattern TRAILING_ZEROES = Pattern.compile("0*$");
+  private static final Pattern ENDING_WITH_DOT = Pattern.compile("\\.$");
+
+  public static String getFormattedDate(LocalDateTime date) {


It would be nice to have a Javadoc here as well

…14884_POC-JSON-EXTRACTOR

sonarcloud · 2024-06-25T08:54:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

gnespolino added the run-api-tests This label will trigger an api-test job for the PR. label May 25, 2023

gnespolino requested review from d-bernat and maikelarabori May 25, 2023 14:14

gnespolino marked this pull request as draft May 25, 2023 14:14

gnespolino changed the title ~~feat: extract enrollment and event values from json [DHIS2-14884]~~ feat: [WIP] extract enrollment and event values from json [DHIS2-14884] May 25, 2023

d-bernat approved these changes May 25, 2023

View reviewed changes

maikelarabori added the run-api-analytics-tests Enables analytics e2e tests label Jun 7, 2023

maikelarabori reviewed Jun 7, 2023

View reviewed changes

style: formatting

e6046d3

gnespolino force-pushed the DHIS2-14884_POC-JSON-EXTRACTOR branch from 264dded to e6046d3 Compare July 18, 2023 09:01

gnespolino and others added 4 commits July 18, 2023 13:20

feat: peer review comments [DHIS2-14884]

bbaa4b8

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

a5bd06d

test: fixing tests [DHIS2-14884]

58b2470

test: fixing tests [DHIS2-14884]

0d86bf5

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

438fe7f

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

881820f

gnespolino added 4 commits October 24, 2023 16:28

Merge remote-tracking branch 'origin/master' into DHIS2-14884_POC-JSO…

b433b25

…N-EXTRACTOR # Conflicts: # dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/TeiAnalyticsQueryService.java

Merge remote-tracking branch 'origin/master' into DHIS2-14884_POC-JSO…

59413bd

…N-EXTRACTOR

feat: more fields extracted from json [DHIS2-14884]

1c14d49

feat: more fields extracted from json [DHIS2-14884]

e330766

github-advanced-security bot found potential problems Oct 25, 2023

View reviewed changes

...ics/src/main/java/org/hisp/dhis/analytics/common/query/jsonextractor/SqlRowSetDelegator.java Dismissed Show dismissed Hide dismissed

gnespolino added 2 commits October 25, 2023 17:40

feat: fixing warning [DHIS2-14884]

4fc487f

Merge remote-tracking branch 'origin/master' into DHIS2-14884_POC-JSO…

76e3b47

…N-EXTRACTOR

gnespolino changed the title ~~feat: [WIP] extract enrollment and event values from json [DHIS2-14884]~~ feat: [WIP] extract enrollment and event values from json [DHIS2-14884] - long standing DONT close please Oct 30, 2023

gnespolino added 2 commits October 30, 2023 11:07

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

3f79385

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

636d06c

gnespolino and others added 13 commits June 6, 2024 14:07

feat: rowcontext from json [DHIS2-14884]

76c1bf4

feat: rowcontext from json [DHIS2-14884]

0bfe8a0

feat: conditions are now exists subquery, left joins removed [DHIS2-1…

0f8a60b

…4884]

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

d77d9c1

feat: conditions are now exists subquery, left joins removed [DHIS2-1…

0857234

…4884]

Merge remote-tracking branch 'origin/DHIS2-14884_POC-JSON-EXTRACTOR' …

8d32221

…into DHIS2-14884_POC-JSON-EXTRACTOR

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

9117f1b

fix: fixes some small issues (dates and SCHEDULE)[DHIS2-14884]

f044708

test: makes some e2e more consistent [DHIS2-14884]

be1129c

fix: fixes some small issues (dates and SCHEDULE)[DHIS2-14884]

1f1b23e

test: makes some e2e more consistent [DHIS2-14884]

637f058

test: fixes unit test [DHIS2-14884]

6d4022e

test: adds date formatter test [DHIS2-14884]

6b229fd

github-advanced-security bot found potential problems Jun 11, 2024

View reviewed changes

maikelarabori reviewed Jun 12, 2024

View reviewed changes

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

53f7256

gnespolino marked this pull request as ready for review June 12, 2024 11:39

gnespolino changed the title ~~feat: [WIP] Improve TEI Performance [DHIS2-14884]~~ feat: Improve TEI Performance [DHIS2-14884] Jun 12, 2024

gnespolino added 3 commits June 12, 2024 13:40

Merge remote-tracking branch 'refs/remotes/origin/master' into DHIS2-…

890f99f

…14884_POC-JSON-EXTRACTOR

fix: missing import after merge [DHIS2-14884]

de6035b

fix: missing import after merge [DHIS2-14884]

db33d6e

maikelarabori approved these changes Jun 12, 2024

View reviewed changes

gnespolino and others added 5 commits June 17, 2024 09:47

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

d5329b6

fix: fixes NPE [DHIS2-17577]

ad9284c

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

7043a9d

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

09a5007

Merge branch 'master' into DHIS2-14884_POC-JSON-EXTRACTOR

76ceae0

gnespolino merged commit 100eb77 into master Jun 26, 2024
15 checks passed

gnespolino deleted the DHIS2-14884_POC-JSON-EXTRACTOR branch June 26, 2024 15:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Improve TEI Performance [DHIS2-14884] #14039

feat: Improve TEI Performance [DHIS2-14884] #14039

gnespolino commented May 25, 2023 •

edited by maikelarabori

Loading

maikelarabori left a comment

maikelarabori Jun 7, 2023

gnespolino Jul 18, 2023

maikelarabori left a comment •

edited

Loading

codecov bot commented Jul 18, 2023 •

edited

Loading

sonarcloud bot commented Jul 20, 2023

sonarcloud bot commented Aug 21, 2023

maikelarabori Jun 12, 2024 •

edited

Loading

sonarcloud bot commented Jun 25, 2024

feat: Improve TEI Performance [DHIS2-14884] #14039

feat: Improve TEI Performance [DHIS2-14884] #14039

Conversation

gnespolino commented May 25, 2023 • edited by maikelarabori Loading

maikelarabori left a comment

Choose a reason for hiding this comment

maikelarabori Jun 7, 2023

Choose a reason for hiding this comment

gnespolino Jul 18, 2023

Choose a reason for hiding this comment

maikelarabori left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Jul 18, 2023 • edited Loading

Codecov Report

sonarcloud bot commented Jul 20, 2023

sonarcloud bot commented Aug 21, 2023

maikelarabori Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Jun 25, 2024

Quality Gate passed

gnespolino commented May 25, 2023 •

edited by maikelarabori

Loading

maikelarabori left a comment •

edited

Loading

codecov bot commented Jul 18, 2023 •

edited

Loading

maikelarabori Jun 12, 2024 •

edited

Loading