informatics-isi-edu / ermrestjs

ERMrest client library in JavaScript
Apache License 2.0
4 stars 3 forks source link

Use inner join for pseudo-column aggregate requests #923

Closed RFSH closed 2 years ago

RFSH commented 2 years ago

While discussing the changes required for #685 we noticed that I implemented the pseudo-column aggregates with outer join.

I couldn't find any explanation as to why we decided to use outer join and there really is no benefit to using outer join. So we decided to rewrite the logic a bit to use inner join which is more performant and wouldn't cause other issues (Combining outer join with filter in source could be problematic).

Therefore instead of doing

<pseudo-column table url>/<join path based on reverse of given source>/<key value pairs of all the shortestkey values>/<shortest key>;<aggregate column>

I changed the code to do

<main table url>/<key value pairs of all the shortestkey values>/<join path based on the given source>/<shortest key>;<aggregate column>

P.S. The getAggregatedValue function returns a promise that is resolved with the results and therefore our test cases are just making sure the result is correct (and not testing the actual ermrest request). That's why this PR only includes code changes.