I wanted to take a look at this thread and do a review, but it's not very clear to me if the recent patches posted here are independent or how exactly they fit together. I see
1) hashagg-20200212-1.patch (2020/02/13 by Jeff)
2) refactor.patch (2020/02/13 by Jeff)
3) v1-0001-aggregated-unaggregated-cols-together.patch (2020/02/14 by Melanie)
I suppose this also confuses the cfbot - it's probably only testing (3) as it's the last thing posted here, at least I think that's the case.
And it fails:
nodeAgg.c: In function ‘find_aggregated_cols_walker’: nodeAgg.c:1208:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] FindColsContext *find_cols_context = (FindColsContext *) context; ^ nodeAgg.c: In function ‘find_unaggregated_cols_walker’: nodeAgg.c:1225:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] FindColsContext *find_cols_context = (FindColsContext *) context; ^ cc1: all warnings being treated as errors <builtin>: recipe for target 'nodeAgg.o' failed make[3]: *** [nodeAgg.o] Error 1 make[3]: *** Waiting for unfinished jobs....
Oops! Sorry, I would fix the code that those compiler warnings is complaining about, but that would confuse the cfbot more. So, I'll let Jeff decide what he wants to do about the patch at all (e.g. include it in his overall patch or exclude it for now). Anyway it is trivial to move those declarations up, were he to decide to include it.