search

openedx / aspects-dbt

The dbt project for Open edX Aspects!
Apache License 2.0
2 stars 5 forks source link

feat: Speed up problem results by combining responses into one CTE #35

Closed bmtcril closed 9 months ago

bmtcril commented 10 months ago

This change was based on a recommendation from Altinity, tests showed about a 30% speedup but I'm not 100% sure it's correct on a larger dataset so putting it up for comment while I test.

bmtcril commented 10 months ago

@SoryRawyer what do you think of this? If it's good I think we can do the same here: https://github.com/openedx/tutor-contrib-aspects/blob/4ea65d69307b53e5da96cedb46296c785fe3a259/tutoraspects/templates/openedx-assets/queries/fact_learner_problem_summary.sql#L1

bmtcril commented 9 months ago

@SoryRawyer this is out of date, but I think we should still look at making these changes anywhere we're doing multiple CTEs and can combine them. It was a pretty marked performance difference.

SoryRawyer commented 9 months ago

Ah sorry for letting this get past me, I must have thought this was shelved for a while. At first pass it looks good but I'll take a deeper look.

SoryRawyer commented 9 months ago

@bmtcril did Altinity say anything about using window functions in views? I might be missing something, but I'm not sure that was ever resolved. That said, I think this is a good strategy for some of the more complicated Superset datasets like the one you linked to.