-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: optimizer rewrite #5676
feat: optimizer rewrite #5676
Changes from all commits
c31e0d0
5c69559
3769aa2
86db3f9
4dd76c0
6ca3626
d3fb48b
62b26ff
b38f5e2
b6e9a94
cde7095
9f81cb7
8fe72e3
2137ddd
c9e2a8f
e3c41dc
9eae352
c744ea9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ | |
from snuba.query.processors.logical.filter_in_select_optimizer import ( | ||
FilterInSelectOptimizer, | ||
) | ||
from snuba.query.query_settings import QuerySettings | ||
from snuba.query.query_settings import HTTPQuerySettings, QuerySettings | ||
from snuba.query.snql.anonymize import format_snql_anonymized | ||
from snuba.query.snql.parser import ( | ||
MAX_LIMIT, | ||
|
@@ -49,7 +49,7 @@ | |
_replace_time_condition, | ||
_treeify_or_and_conditions, | ||
) | ||
from snuba.state import explain_meta | ||
from snuba.state import explain_meta, get_int_config | ||
from snuba.util import parse_datetime | ||
from snuba.utils.constants import GRANULARITIES_AVAILABLE | ||
|
||
|
@@ -1064,19 +1064,12 @@ def transform(exp: Expression) -> Expression: | |
query.transform_expressions(transform) | ||
|
||
|
||
def optimize_filter_in_select( | ||
query: CompositeQuery[QueryEntity] | LogicalQuery, | ||
) -> None: | ||
FilterInSelectOptimizer().process_mql_query(query) | ||
|
||
|
||
CustomProcessors = Sequence[ | ||
Callable[[Union[CompositeQuery[QueryEntity], LogicalQuery]], None] | ||
] | ||
|
||
MQL_POST_PROCESSORS: CustomProcessors = POST_PROCESSORS + [ | ||
quantiles_to_quantile, | ||
optimize_filter_in_select, | ||
] | ||
|
||
|
||
|
@@ -1116,6 +1109,17 @@ def parse_mql_query( | |
settings, | ||
) | ||
|
||
# Filter in select optimizer | ||
feat_flag = get_int_config("enable_filter_in_select_optimizer", default=1) | ||
if feat_flag: | ||
with sentry_sdk.start_span( | ||
op="processor", description="filter_in_select_optimize" | ||
): | ||
if settings is None: | ||
FilterInSelectOptimizer().process_query(query, HTTPQuerySettings()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to point out that in this context settings can be None, LogicalQueryProcessor interface requires settings not None, thus I had to create dummy object. Since settings arent used by my processor it doesnt matter, but the fact that i have to create this dummy instead of None smells to me |
||
else: | ||
FilterInSelectOptimizer().process_query(query, settings) | ||
|
||
# Custom processing to tweak the AST before validation | ||
with sentry_sdk.start_span(op="processor", description="custom_processing"): | ||
if custom_processing is not None: | ||
|
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.
couldnt use this function in dsl.py bc of circular import with conditions.py. I thought this belonged in dsl.py anyways so I moved it.