Date: Tue, 18 Feb 2020 19:57:49 +0100
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Jeff Davis <pgsql@j-davis.com>,
	Taylor Vesely <tvesely@pivotal.io>, Adam Lee <ali@pivotal.io>,
	Melanie Plageman <mplageman@pivotal.io>,
	Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Memory-Bounded Hash Aggregation
Message-ID: <20200218185749.srk733n4z57lzxf2@development>
References: <9be86fc1adc315f69b8af4b379087ab451008d8a.camel@j-davis.com>
 <20190802064405.GA94036@mars.local>
 <617fea8258ba3df7bc8b21cd7f3afc219f2383d9.camel@j-davis.com>
 <20190802152123.ichmacgx3y3x7dc3@development>
 <CAFaX_4Ls5UGQ2UDYkWhn2xFq=V6BKpUbKSQnWmy27VWhQ2=enA@mail.gmail.com>
 <e52707d4665e32867fb6dd181825ef15f853b8bf.camel@j-davis.com>
 <20191214173225.3kzsbncr6brpxdll@development>
 <d72e494db7b075ef02c3163e6614e698701d2c3e.camel@j-davis.com>
 <775d754c-9a61-5363-6998-f87d15f1b592@iki.fi>
 <CAAKRu_aefEsv+UkQWqu+ioEnoiL2LJu9Diuh9BR8MbyXuZ0j4A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAAKRu_aefEsv+UkQWqu+ioEnoiL2LJu9Diuh9BR8MbyXuZ0j4A@mail.gmail.com>
Precedence: bulk

Hi,

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....


It's probably a good idea to either start a separate thread for patches
that are only loosely related to the main topic, or always post the
whole patch series.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services