Subject: Re: explain plans with information about (modified) gucs
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 legrand legrand <legrand_legrand@hotmail.com>, pgsql-hackers@postgresql.org
References: <e1791b4c-df9c-be02-edc5-7c8874944be0@2ndquadrant.com>
 <10561.1544800904@sss.pgh.pa.us>
 <c1282df1-43a8-0854-dfee-790d7dd9c93a@2ndquadrant.com>
 <631ae5c8-5581-6c5e-382c-2dcdb2e5c956@2ndquadrant.com>
 <1545083780942-0.post@n3.nabble.com>
 <1edcee55-ea96-d123-199d-4c5f1fc5003b@2ndquadrant.com>
 <24065.1545085015@sss.pgh.pa.us>
 <2c09fe19-e4a0-ea66-5a7f-e69d46420e28@2ndquadrant.com>
 <20181217234353.fde7vityxvoullil@alap3.anarazel.de>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Message-ID: <f47d7590-cd48-38e3-229d-c60c0f4345f1@2ndquadrant.com>
Date: Tue, 18 Dec 2018 15:58:48 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.3.1
MIME-Version: 1.0
In-Reply-To: <20181217234353.fde7vityxvoullil@alap3.anarazel.de>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/18/18 12:43 AM, Andres Freund wrote:
> Hi,
> 
> On 2018-12-18 00:38:16 +0100, Tomas Vondra wrote:
>> On 12/17/18 11:16 PM, Tom Lane wrote:
>>> Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>>>> Yeah, I've been thinking about that too. Currently it gets filtered out
>>>> because it's in the CLIENT_CONN_STATEMENT group, but the code only
>>>> includes QUERY_TUNING_*.
>>>> But we don't want to include everything from CLIENT_CONN_STATEMENT,
>>>> because that would include various kinds of timeouts, vacuuming
>>>> parameters, etc.
>>>> And the same issue applies to work_mem, which is in RESOURCES_MEM. And
>>>> of course, there's a lot of unrelated stuff in RESOURCES_MEM.
>>>> So I guess we'll need to enable QUERY_TUNING_* and then selectively a
>>>> couple of individual options from other groups.
>>>
>>> This is putting way too much policy into the mechanism, if you ask me.
>>>
>>
>> Doesn't that depend on how it'd be implemented? I have not envisioned to
>> make these decisions in explain.c, but rather to keep them in guc.c
>> somehow. Say in a function like this:
>>
>>     bool guc_affects_query_planning(config_generic *conf);
>>
>> which would be a fairly simple check outlined before (QUERY_TUNING_*
>> plus a couple of individual GUCs). Other use cases might provide similar
>> filters.
> 
> If we were to do that, I'd suggest implementing a GUC_* flag specified
> in the definition of the GUC, rather than a separate function containing
> all the knowledge (but such a function could obviously still be used to
> return such a fact).
> 

Seems reasonable.

> 
>> An alternative would be to somehow track this in the GUC definitions
>> directly (similarly to how we track the group), but that seems rather
>> inflexible and I'm not sure how would that handle ad-hoc use cases.
> 
> Not sure what problem you see here?
> 

My main concern with that was how many flags could we need, if we use
this as the way to implement this and similar use cases. There are 32
bits available, and 24 of them are already used for GUC_* flags. So if
we use this as an "official" way to support similar use cases, we could
run out of space pretty fast.

The callback would also allow ad hoc filtering, which is not very
practical from extensions (e.g. you can't define a new flag, because it
might conflict with something defined in another extension).

But I think that's nonsense - so far we have not seen any such use case,
so it's pretty pointless to design for it. If that changes and some new
use case is proposed in the future, we can rethink this based on it.

I'll go with a new flag, marking all GUCs related to query planning, and
post a new patch soon.

regards

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