-
Notifications
You must be signed in to change notification settings - Fork 5
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
improve puvspr.dat queries #397
Conversation
This pull request is being automatically deployed with Vercel (learn more). marxan – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan/DyksnAJzWiQ9nqEpFXPUDaWBLmu8 marxan-storybook – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/CBXsULxxj6GpWghVPE4PiJVSS3bV |
779db1c
to
b482ed9
Compare
d11064d
to
492710b
Compare
492710b
to
da9c477
Compare
c625998
to
d82b287
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR seems to mix three different ones (deno bot, calculating protection status and improving puvspr dat). Would be great to clean it up first before jumping into review. Thanks!
da9c477
to
89458eb
Compare
d82b287
to
13c0772
Compare
89458eb
to
96fcc8c
Compare
96fcc8c
to
11b1941
Compare
@InjectConnection(DbConnections.geoprocessingDB) | ||
private readonly connection: Connection, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can just use puvsprRepo.query
instead creating new connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, I know - I did so because with this PR the puvsprRepo
should become redundant and we should be able to drop it altogether: I left it in the current PR in order not to change too much in case the new approach ends up not working out in practice, but all going well we should be able to drop the repo.
sorry about the PR stack mess - this is properly isolated now 😬 |
as discussed with Alicia - implementation based on view (which I have kept for the moment) does not allow (as far as I can think...) to prefilter by
scenarioId
- with the implementation proposed in this PR I have been able to get query times down from ~4min with test data to ~20secs (most of the deal is actually indexes, but still, we also filter out data we won't need)