opensafely-core / ehrql

ehrQL: the electronic health record query language for OpenSAFELY
https://docs.opensafely.org/ehrql/
Other
7 stars 3 forks source link

Refactor query model transformation code #573

Closed evansd closed 1 year ago

evansd commented 2 years ago

In order to facilitate efficient execution of queries, or to make parts of the query engine code simpler, we want to be able to rewrite the query model structure given to us by the user in various ways.

We currently do this in fairly inelegant fashion, by duplicating the entire structure and then updating bits of it in place. This has already led to one bug.

We should take a different approach here, probably using something like a zipper data structure.

This is not urgent, but it will become increasingly important to get this right as we start adding more sorts of transformation.

inglesp commented 1 year ago

@evansd can you update the description in light of #1003?

evansd commented 1 year ago

I was going to wait until that was merged in case there turned out to be some show-stopping flaw in my approach and we had to resort the nuclear option (a shared cache of hashes which can be globally invalidated).

evansd commented 1 year ago

Closing this issue following the work done in:

I'm not claiming that's the last word in query transformation, but it does mean that this code no longer stands out as a thing which needs refactoring over and above other things we'd like to improve.