-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Currently it's very hard to follow the logic in the query expression compiler. The use of the visitor pattern means that calls bounce around between different methods on different visitor classes in a way that's difficult to follow. For example, the call stack to compile a simple count query is:
session.py(1124)evaluate()
session.py(1003)_compile_and_get_info()
_query_expr_compiler/_compiler.py(165)__call__()
_query_expr.py(818)accept()
_query_expr_compiler/_base_measurement_visitor.py(1091)visit_groupby_count()
_query_expr_compiler/_base_measurement_visitor.py(496)_build_adaptive_groupby_agg_and_noise_info()
_query_expr_compiler/_base_measurement_visitor.py(1078)_build_groupby_agg_from_groupby()
_query_expr_compiler/_base_measurement_visitor.py(1033)build_groupby_count()
tmlt/core/measurements/aggregations.py(186)create_count_measurement()
This gets much worse as you make the query more complicated.
It doesn't seem to me like there's a terribly strong reason to use the visitor pattern here - it would be a good choice if we wanted to make it easy for other developers to add new types of program that QueryExprs can be compiled into, but I don't think that's a design goal. So I think we could substantially simplify things by moving the compilation logic onto the QueryExprs. Instead of the current .accept() method, QueryExprs would have a .compile(...) method, which would build and return the Core measurement directly.