Reject unsupported aggregates during anon query validation. #70
Conversation
src/query/validation.c
Outdated
|
|
||
| static void verify_aggregators(Query *query) | ||
| { | ||
| query_tree_walker(query, verify_aggregator, NULL, QTW_DONT_COPY_QUERY); |
There was a problem hiding this comment.
QTW_DONT_COPY_QUERY has no effect because this is not a mutator. I think 0 can be passed as flags.
src/query/validation.c
Outdated
| if (IsA(node, Query)) | ||
| return query_tree_walker((Query *)node, verify_aggregator, context, QTW_DONT_COPY_QUERY); |
There was a problem hiding this comment.
There's a query_or_expression_tree_walker in nodeFuncs which could handle the checking for us.
bool query_or_expression_tree_walker(Node *node, bool (*walker) (), void *context, int flags) {
if (node && IsA(node, Query))
return query_tree_walker((Query *) node, walker, context, flags);
else
return walker(node, context);
}There was a problem hiding this comment.
This is only useful for starting the inspection when we don't know if the node is a query or an expression.
We already know we are starting from a query node, so we don't need to check anything at that point.
There was a problem hiding this comment.
The tail call could be replaced with that and the condition could be dropped. Anyhow we don't gain much with this change so it's fine.
There was a problem hiding this comment.
No, it can't be used there, it would go into an infinite loop (it calls the walker directly instead of expression_tree_walker).
|
Ready for review. |
Closes #14.