CNF/Constraint optimizer in new analyzer#47617
Conversation
4a82c02 to
dbf0ee9
Compare
dbf0ee9 to
0f6e4d3
Compare
| void ConvertQueryToCNFPass::run(QueryTreeNodePtr query_tree_node, ContextPtr context) | ||
| { | ||
| const auto & settings = context->getSettingsRef(); | ||
| if (!settings.convert_query_to_cnf) |
There was a problem hiding this comment.
This check better should be done in context of every QueryNode
There was a problem hiding this comment.
why? don't we use the same context for all of them?
| }; | ||
|
|
||
| optimize_filter(query_node->getWhere()); | ||
| optimize_filter(query_node->getPrewhere()); |
There was a problem hiding this comment.
We don't do it for HAVING clause?
There was a problem hiding this comment.
in the original implementation, it was done only for WHERE clause so I just went with that.
HAVING is applied to aggregate values, while constraints are defined for each table, i.e. we can't have constraints for aggregate.
But I can actually transform it to CNF so I will add it.
There was a problem hiding this comment.
IMO, it should work for any (ok, except steps after aggregation) part of query, for example in SELECT part as well.
#33544
|
|
||
| auto & new_function_node = current->as<FunctionNode &>(); | ||
| function_node->getArguments().getNodes() = std::move(new_function_node.getArguments().getNodes()); | ||
| function_node->resolveAsFunction(function_resolver); |
There was a problem hiding this comment.
why not *function_node = std::move(new_function_node)?
There was a problem hiding this comment.
Definition of implicit copy assignment operator for 'IQueryTreeNode' is deprecated because it has a user-declared destructor (clang -Wdeprecated-copy-with-dtor)
src/Analyzer/Passes/CNF.cpp
Outdated
| { | ||
| return node_with_hash.hash == rhs.node_with_hash.hash | ||
| ? negative < rhs.negative | ||
| : node_with_hash.hash < rhs.node_with_hash.hash; |
| { | ||
| transformAtoms([&](const AtomicFormula & atom) | ||
| { | ||
| static const std::unordered_map<std::string, std::string> inverse_relations = { |
There was a problem hiding this comment.
Seems that this map must be the same as in pushNotIntoFunction
There was a problem hiding this comment.
we want only a subset - function starting with not and comparisons that are not less or equals
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Add CNF/constraint optimizer in new analyzer.
Documentation entry for user-facing changes