Horton

Thread

Commits

  1. Add plan_cache_mode setting

  2. Add some noreturn attributes to help static analyzers

  3. document when PREPARE uses generic plans

  4. Redesign the plancache mechanism for more flexibility and efficiency.

  1. PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-01-23T20:10:49Z

    Hi,

this patch is based on discussions related to plpgsql2 project.

Currently we cannot to control plan cache from plpgsql directly. We can use
dynamic SQL if we can enforce oneshot plan - but it means little bit less
readable code (if we enforce dynamic SQL from performance reasons). It
means so the code cannot be checked by plpgsql check too.

The plan cache subsystem allows some control by options
CURSOR_OPT_GENERIC_PLAN and CURSOR_OPT_CUSTOM_PLAN. So we just a interface
how to use these options from PLpgSQL. I used Ada language feature (used in
PL/SQL too) - PRAGMA statement. It allows to set compiler directives. The
syntax of PRAGMA statements allows to set a level where entered compiler
directive should be applied. It can works on function level or block level.

Attached patch introduces PRAGMA plan_cache with options: DEFAULT,
FORCE_CUSTOM_PLAN, FORCE_GENERIC_PLAN. Plan cache is partially used every
time - the parser/analyzer result is cached every time.

Examples:

CREATE OR REPLACE FUNCTION foo(a int)
RETURNS int AS  $$
DECLARE ..
BEGIN

   DECLARE
     /* block level (local scope) pragma */
     PRAGMA plan_cache(FORCE_CUSTOM_PLAN);
   BEGIN
     SELECT /* slow query - dynamic sql is not necessary */
   END;

 END;

Benefits:

1. remove one case where dynamic sql is necessary now - security, static
check
2. introduce PRAGMAs - possible usage: autonomous transactions, implicit
namespaces settings (namespace for auto variables, namespace for function
arguments).

Comments, notes?

Regards

Pavel

  2. Re: PoC plpgsql - possibility to force custom or generic plan

    Jim Nasby <jim.nasby@bluetreble.com> — 2017-01-23T20:59:32Z

    On 1/23/17 2:10 PM, Pavel Stehule wrote:
> Comments, notes?

+1 on the idea. It'd also be nice if we could expose control of plans 
for dynamic SQL, though I suspect that's not terribly useful without 
some kind of global session storage.

A couple notes on a quick read-through:

Instead of paralleling all the existing namespace stuff, I wonder if 
it'd be better to create explicit block infrastructure. AFAIK PRAGMAs 
are going to have a lot of the same requirements (certainly the nesting 
is the same), and we might want more of this king of stuff in the 
future. (I've certainly wished I could set a GUC in a plpgsql block and 
have it's settings revert when exiting the block...)

Perhaps that's as simple as renaming all the existing _ns_* functions to 
_block_ and then adding support for pragmas...

Since you're adding cursor_options to PLpgSQL_expr it should probably be 
removed as an option to exec_*.

finit_ would be better named free_.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

  3. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-01-24T05:38:45Z

    Hi

2017-01-23 21:59 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

> On 1/23/17 2:10 PM, Pavel Stehule wrote:
>
>> Comments, notes?
>>
>
> +1 on the idea. It'd also be nice if we could expose control of plans for
> dynamic SQL, though I suspect that's not terribly useful without some kind
> of global session storage.
>
> A couple notes on a quick read-through:
>
> Instead of paralleling all the existing namespace stuff, I wonder if it'd
> be better to create explicit block infrastructure. AFAIK PRAGMAs are going
> to have a lot of the same requirements (certainly the nesting is the same),
> and we might want more of this king of stuff in the future. (I've certainly
> wished I could set a GUC in a plpgsql block and have it's settings revert
> when exiting the block...)
>

I am not sure if I understand. ?? Setting GUC by PRAGMA can work - the
syntax supports it and GUC API supports nesting. Not sure about exception
handling - but it should not be problem probably.

Please, can you show some examples.


> Perhaps that's as simple as renaming all the existing _ns_* functions to
> _block_ and then adding support for pragmas...
>
> Since you're adding cursor_options to PLpgSQL_expr it should probably be
> removed as an option to exec_*.
>

I have to recheck it. Some cursor options going from dynamic cursor
variables and are related to dynamic query - not query that creates query
string.

>
> finit_ would be better named free_.


good idea

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
> 855-TREBLE2 (855-873-2532)
>

  4. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-01-24T05:43:41Z

    2017-01-23 21:59 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

> On 1/23/17 2:10 PM, Pavel Stehule wrote:
>
>> Comments, notes?
>>
>
> +1 on the idea. It'd also be nice if we could expose control of plans for
> dynamic SQL, though I suspect that's not terribly useful without some kind
> of global session storage.
>

yes - it requires classic normalised query string controlled plan cache.

But same plan cache options can be valid for prepared statements - probably
with GUC. This is valid theme, but out of my proposal in this moment.

Regards

Pavel

  5. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-01-24T20:33:56Z

    2017-01-24 6:38 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

> Hi
>
> 2017-01-23 21:59 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
>
>> On 1/23/17 2:10 PM, Pavel Stehule wrote:
>>
>>> Comments, notes?
>>>
>>
>> +1 on the idea. It'd also be nice if we could expose control of plans for
>> dynamic SQL, though I suspect that's not terribly useful without some kind
>> of global session storage.
>>
>> A couple notes on a quick read-through:
>>
>> Instead of paralleling all the existing namespace stuff, I wonder if it'd
>> be better to create explicit block infrastructure. AFAIK PRAGMAs are going
>> to have a lot of the same requirements (certainly the nesting is the same),
>> and we might want more of this king of stuff in the future. (I've certainly
>> wished I could set a GUC in a plpgsql block and have it's settings revert
>> when exiting the block...)
>>
>
> I am not sure if I understand. ?? Setting GUC by PRAGMA can work - the
> syntax supports it and GUC API supports nesting. Not sure about exception
> handling - but it should not be problem probably.
>
> Please, can you show some examples.
>
>
>> Perhaps that's as simple as renaming all the existing _ns_* functions to
>> _block_ and then adding support for pragmas...
>>
>> Since you're adding cursor_options to PLpgSQL_expr it should probably be
>> removed as an option to exec_*.
>>
>
> I have to recheck it. Some cursor options going from dynamic cursor
> variables and are related to dynamic query - not query that creates query
> string.
>

hmm .. so current state is better due using options like
CURSOR_OPT_PARALLEL_OK

     if (expr->plan == NULL)
        exec_prepare_plan(estate, expr, (parallelOK ?
                          CURSOR_OPT_PARALLEL_OK : 0) |
expr->cursor_options);

This options is not permanent feature of expression - and then I cannot to
remove cursor_option argument from exec_*

I did minor cleaning - remove cursor_options from plpgsql_var

Regards

Pavel



>> finit_ would be better named free_.
>
>
> good idea
>
> Regards
>
> Pavel
>
>
>>
>> --
>> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
>> Experts in Analytics, Data Architecture and PostgreSQL
>> Data in Trouble? Get it in Treble! http://BlueTreble.com
>> 855-TREBLE2 (855-873-2532)
>>
>
>

  6. Re: PoC plpgsql - possibility to force custom or generic plan

    Jim Nasby <jim.nasby@bluetreble.com> — 2017-01-25T20:06:30Z

    On 1/23/17 11:38 PM, Pavel Stehule wrote:
>
>     Instead of paralleling all the existing namespace stuff, I wonder if
>     it'd be better to create explicit block infrastructure. AFAIK
>     PRAGMAs are going to have a lot of the same requirements (certainly
>     the nesting is the same), and we might want more of this king of
>     stuff in the future. (I've certainly wished I could set a GUC in a
>     plpgsql block and have it's settings revert when exiting the block...)
>
>
> I am not sure if I understand. ?? Setting GUC by PRAGMA can work - the
> syntax supports it and GUC API supports nesting. Not sure about
> exception handling - but it should not be problem probably.
>
> Please, can you show some examples.

 From a code standpoint, there's already some ugliness around blocks: 
there's the code that handles blocks themselves (which IIRC is 
responsible for subtransactions), then there's the namespace code, which 
is very separate even though namespaces are very much tied to blocks. 
Your patch is adding another layer into the mix, separate from both 
blocks and namespaces. I think it would be better to combine all 3 
together, or at least not make matters worse. So IMHO the pragma stuff 
should be part of handling blocks, and not something that's stand alone. 
IE: make the pragma info live in PLpgSQL_stmt_block.

GUCs support SET LOCAL, but that's not the same as local scoping because 
the setting stays in effect unless the substrans aborts. What I'd like 
is the ability to set a GUC in a plpgsql block *and have the setting 
revert on block exit*.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

  7. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-01-25T21:09:40Z

    2017-01-25 21:06 GMT+01:00 Jim Nasby <Jim.Nasby@bluetreble.com>:

> On 1/23/17 11:38 PM, Pavel Stehule wrote:
>
>>
>>     Instead of paralleling all the existing namespace stuff, I wonder if
>>     it'd be better to create explicit block infrastructure. AFAIK
>>     PRAGMAs are going to have a lot of the same requirements (certainly
>>     the nesting is the same), and we might want more of this king of
>>     stuff in the future. (I've certainly wished I could set a GUC in a
>>     plpgsql block and have it's settings revert when exiting the block...)
>>
>>
>> I am not sure if I understand. ?? Setting GUC by PRAGMA can work - the
>> syntax supports it and GUC API supports nesting. Not sure about
>> exception handling - but it should not be problem probably.
>>
>> Please, can you show some examples.
>>
>
> From a code standpoint, there's already some ugliness around blocks:
> there's the code that handles blocks themselves (which IIRC is responsible
> for subtransactions), then there's the namespace code, which is very
> separate even though namespaces are very much tied to blocks. Your patch is
> adding another layer into the mix, separate from both blocks and
> namespaces. I think it would be better to combine all 3 together, or at
> least not make matters worse. So IMHO the pragma stuff should be part of
> handling blocks, and not something that's stand alone. IE: make the pragma
> info live in PLpgSQL_stmt_block.
>

I don't think it is fully correct - the pragma can be related to function
too - and namespaces can be related to some other statements - cycles. Any
PLpgSQL_stmt_block does some overhead and probably we want to build a fake
statements to ensure 1:1 relations between namespaces and blocks.

I didn't implement and proposed third level of pragma - statement. For
example the assertions in Ada language are implemented with pragma.
Currently I am not thinking about this form for Postgres.

The cursor options is better stored in expression - the block related GUC
probably should be stored in stmt_block. The pragma is  additional
information, and how this information will be used and what impact will be
on generated code depends on pragma - can be different.


> GUCs support SET LOCAL, but that's not the same as local scoping because
> the setting stays in effect unless the substrans aborts. What I'd like is
> the ability to set a GUC in a plpgsql block *and have the setting revert on
> block exit*.


I am think so it is solvable.

Regards

Pavel

>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
> 855-TREBLE2 (855-873-2532)
>

  8. Re: PoC plpgsql - possibility to force custom or generic plan

    Greg Stark <stark@mit.edu> — 2017-01-27T12:14:22Z

    On 25 January 2017 at 20:06, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
> GUCs support SET LOCAL, but that's not the same as local scoping because the
> setting stays in effect unless the substrans aborts. What I'd like is the
> ability to set a GUC in a plpgsql block *and have the setting revert on
> block exit*.

I'm wondering which GUCs you have in mind to use this with.

Because what you're describing is dynamic scoping and I'm wondering if
what you're really looking for is lexical scoping. That would be more
in line with how PL/PgSQL variables are scoped and with how #pragmas
usually work. But it would probably not be easy to reconcile with how
GUCs work.

-- 
greg

  9. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-02-01T20:59:10Z

    Hi

2017-01-24 21:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

>
>
>
>>
>>> Perhaps that's as simple as renaming all the existing _ns_* functions to
>>> _block_ and then adding support for pragmas...
>>>
>>> Since you're adding cursor_options to PLpgSQL_expr it should probably be
>>> removed as an option to exec_*.
>>>
>>
>> I have to recheck it. Some cursor options going from dynamic cursor
>> variables and are related to dynamic query - not query that creates query
>> string.
>>
>
> hmm .. so current state is better due using options like
> CURSOR_OPT_PARALLEL_OK
>
>      if (expr->plan == NULL)
>         exec_prepare_plan(estate, expr, (parallelOK ?
>                           CURSOR_OPT_PARALLEL_OK : 0) |
> expr->cursor_options);
>
> This options is not permanent feature of expression - and then I cannot to
> remove cursor_option argument from exec_*
>
> I did minor cleaning - remove cursor_options from plpgsql_var
>
> Regards
>
> Pavel
>


+ basic doc

Regards

Pavel

  10. Re: PoC plpgsql - possibility to force custom or generic plan

    Jim Nasby <jim.nasby@bluetreble.com> — 2017-02-01T21:45:24Z

    On 1/27/17 4:14 AM, Greg Stark wrote:
> On 25 January 2017 at 20:06, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
>> GUCs support SET LOCAL, but that's not the same as local scoping because the
>> setting stays in effect unless the substrans aborts. What I'd like is the
>> ability to set a GUC in a plpgsql block *and have the setting revert on
>> block exit*.
>
> I'm wondering which GUCs you have in mind to use this with.

It's been quite some time since I messed with this; the only case I 
remember right now is wanting to do a temporary SET ROLE in an "exec" 
function:

SELECT tools.exec( 'some sql;', role := 'superuser_role' );

To do that, exec has to remember what the current role is and then set 
it back (as well as remembering to do SET LOCAL in case an error happens.

> Because what you're describing is dynamic scoping and I'm wondering if
> what you're really looking for is lexical scoping. That would be more
> in line with how PL/PgSQL variables are scoped and with how #pragmas
> usually work. But it would probably not be easy to reconcile with how
> GUCs work.

Right, because GUCs aren't even simply dynamically scoped; they're 
dynamically scoped with transaction support.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

  11. Re: PoC plpgsql - possibility to force custom or generic plan

    David Steele <david@pgmasters.net> — 2017-03-16T16:15:42Z

    On 2/1/17 3:59 PM, Pavel Stehule wrote:
> Hi
> 
> 2017-01-24 21:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com
> <mailto:pavel.stehule@gmail.com>>:
> 
>             Perhaps that's as simple as renaming all the existing _ns_*
>             functions to _block_ and then adding support for pragmas...
> 
>             Since you're adding cursor_options to PLpgSQL_expr it should
>             probably be removed as an option to exec_*.
> 
>         I have to recheck it. Some cursor options going from dynamic
>         cursor variables and are related to dynamic query - not query
>         that creates query string.  
> 
>     hmm .. so current state is better due using options like
>     CURSOR_OPT_PARALLEL_OK
> 
>          if (expr->plan == NULL)
>             exec_prepare_plan(estate, expr, (parallelOK ?
>                               CURSOR_OPT_PARALLEL_OK : 0) |
>     expr->cursor_options);
> 
>     This options is not permanent feature of expression - and then I
>     cannot to remove cursor_option argument from exec_*
> 
>     I did minor cleaning - remove cursor_options from plpgsql_var
> 
> + basic doc

This patch still applies cleanly and compiles at cccbdde.

Any reviewers want to have a look?

-- 
-David
david@pgmasters.net

  12. Re: PoC plpgsql - possibility to force custom or generic plan

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2017-03-18T18:30:01Z

    On 16/03/17 17:15, David Steele wrote:
> On 2/1/17 3:59 PM, Pavel Stehule wrote:
>> Hi
>>
>> 2017-01-24 21:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com
>> <mailto:pavel.stehule@gmail.com>>:
>>
>>             Perhaps that's as simple as renaming all the existing _ns_*
>>             functions to _block_ and then adding support for pragmas...
>>
>>             Since you're adding cursor_options to PLpgSQL_expr it should
>>             probably be removed as an option to exec_*.
>>
>>         I have to recheck it. Some cursor options going from dynamic
>>         cursor variables and are related to dynamic query - not query
>>         that creates query string.  
>>
>>     hmm .. so current state is better due using options like
>>     CURSOR_OPT_PARALLEL_OK
>>
>>          if (expr->plan == NULL)
>>             exec_prepare_plan(estate, expr, (parallelOK ?
>>                               CURSOR_OPT_PARALLEL_OK : 0) |
>>     expr->cursor_options);
>>
>>     This options is not permanent feature of expression - and then I
>>     cannot to remove cursor_option argument from exec_*
>>
>>     I did minor cleaning - remove cursor_options from plpgsql_var
>>
>> + basic doc
> 
> This patch still applies cleanly and compiles at cccbdde.
> 
> Any reviewers want to have a look?
> 

I'll bite.

I agree with Jim that it's not very nice to add yet another
block/ns-like layer. I don't see why pragma could not be added to either
PLpgSQL_stmt_block (yes pragma can be for whole function but function
body is represented by PLpgSQL_stmt_block as well so no issue there), or
to namespace code. In namespace since they are used for other thing
there would be bit of unnecessary propagation but it's 8bytes per
namespace, does not seem all that much.

My preference would be to add it to PLpgSQL_stmt_block (unless we plan
to add posibility to add pragmas for other loops and other things) but I
am not sure if current block is easily (and in a fast way) accessible
from all places where it's needed. Maybe the needed info could be pushed
to estate from PLpgSQL_stmt_block during the execution.

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

  13. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-03-19T11:32:50Z

    2017-03-18 19:30 GMT+01:00 Petr Jelinek <petr.jelinek@2ndquadrant.com>:

> On 16/03/17 17:15, David Steele wrote:
> > On 2/1/17 3:59 PM, Pavel Stehule wrote:
> >> Hi
> >>
> >> 2017-01-24 21:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com
> >> <mailto:pavel.stehule@gmail.com>>:
> >>
> >>             Perhaps that's as simple as renaming all the existing _ns_*
> >>             functions to _block_ and then adding support for pragmas...
> >>
> >>             Since you're adding cursor_options to PLpgSQL_expr it should
> >>             probably be removed as an option to exec_*.
> >>
> >>         I have to recheck it. Some cursor options going from dynamic
> >>         cursor variables and are related to dynamic query - not query
> >>         that creates query string.
> >>
> >>     hmm .. so current state is better due using options like
> >>     CURSOR_OPT_PARALLEL_OK
> >>
> >>          if (expr->plan == NULL)
> >>             exec_prepare_plan(estate, expr, (parallelOK ?
> >>                               CURSOR_OPT_PARALLEL_OK : 0) |
> >>     expr->cursor_options);
> >>
> >>     This options is not permanent feature of expression - and then I
> >>     cannot to remove cursor_option argument from exec_*
> >>
> >>     I did minor cleaning - remove cursor_options from plpgsql_var
> >>
> >> + basic doc
> >
> > This patch still applies cleanly and compiles at cccbdde.
> >
> > Any reviewers want to have a look?
> >
>
> I'll bite.
>
> I agree with Jim that it's not very nice to add yet another
> block/ns-like layer. I don't see why pragma could not be added to either
> PLpgSQL_stmt_block (yes pragma can be for whole function but function
> body is represented by PLpgSQL_stmt_block as well so no issue there), or
> to namespace code. In namespace since they are used for other thing
> there would be bit of unnecessary propagation but it's 8bytes per
> namespace, does not seem all that much.
>
> My preference would be to add it to PLpgSQL_stmt_block (unless we plan
> to add posibility to add pragmas for other loops and other things) but I
> am not sure if current block is easily (and in a fast way) accessible
> from all places where it's needed. Maybe the needed info could be pushed
> to estate from PLpgSQL_stmt_block during the execution.
>
>
There is maybe partial misunderstand of pragma - it is set of nested
configurations used in compile time only. It can be used in execution time
too - it change nothing.

The pragma doesn't build a persistent tree. It is stack of configurations
that allows fast access to current configuration, and fast leaving of
configuration when the change is out of scope.

I don't see any any advantage to integrate pragma to ns or to stmt_block.
But maybe I don't understand to your idea.

I see a another possibility in code - nesting init_block_directives() to
plpgsql_ns_push and free_block_directives() to plpgsql_ns_pop()

Pavel



> --
>   Petr Jelinek                  http://www.2ndQuadrant.com/
>   PostgreSQL Development, 24x7 Support, Training & Services
>

  14. Re: PoC plpgsql - possibility to force custom or generic plan

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2017-03-19T13:30:28Z

    On 19/03/17 12:32, Pavel Stehule wrote:
> 
> 
> 2017-03-18 19:30 GMT+01:00 Petr Jelinek <petr.jelinek@2ndquadrant.com
> <mailto:petr.jelinek@2ndquadrant.com>>:
> 
>     On 16/03/17 17:15, David Steele wrote:
>     > On 2/1/17 3:59 PM, Pavel Stehule wrote:
>     >> Hi
>     >>
>     >> 2017-01-24 21:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>
>     >> <mailto:pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>>>:
>     >>
>     >>             Perhaps that's as simple as renaming all the existing _ns_*
>     >>             functions to _block_ and then adding support for pragmas...
>     >>
>     >>             Since you're adding cursor_options to PLpgSQL_expr it should
>     >>             probably be removed as an option to exec_*.
>     >>
>     >>         I have to recheck it. Some cursor options going from dynamic
>     >>         cursor variables and are related to dynamic query - not query
>     >>         that creates query string.
>     >>
>     >>     hmm .. so current state is better due using options like
>     >>     CURSOR_OPT_PARALLEL_OK
>     >>
>     >>          if (expr->plan == NULL)
>     >>             exec_prepare_plan(estate, expr, (parallelOK ?
>     >>                               CURSOR_OPT_PARALLEL_OK : 0) |
>     >>     expr->cursor_options);
>     >>
>     >>     This options is not permanent feature of expression - and then I
>     >>     cannot to remove cursor_option argument from exec_*
>     >>
>     >>     I did minor cleaning - remove cursor_options from plpgsql_var
>     >>
>     >> + basic doc
>     >
>     > This patch still applies cleanly and compiles at cccbdde.
>     >
>     > Any reviewers want to have a look?
>     >
> 
>     I'll bite.
> 
>     I agree with Jim that it's not very nice to add yet another
>     block/ns-like layer. I don't see why pragma could not be added to either
>     PLpgSQL_stmt_block (yes pragma can be for whole function but function
>     body is represented by PLpgSQL_stmt_block as well so no issue there), or
>     to namespace code. In namespace since they are used for other thing
>     there would be bit of unnecessary propagation but it's 8bytes per
>     namespace, does not seem all that much.
> 
>     My preference would be to add it to PLpgSQL_stmt_block (unless we plan
>     to add posibility to add pragmas for other loops and other things) but I
>     am not sure if current block is easily (and in a fast way) accessible
>     from all places where it's needed. Maybe the needed info could be pushed
>     to estate from PLpgSQL_stmt_block during the execution.
> 
> 
> There is maybe partial misunderstand of pragma - it is set of nested
> configurations used in compile time only. It can be used in execution
> time too - it change nothing.
> 
> The pragma doesn't build a persistent tree. It is stack of
> configurations that allows fast access to current configuration, and
> fast leaving of configuration when the change is out of scope.
> 
> I don't see any any advantage to integrate pragma to ns or to
> stmt_block. But maybe I don't understand to your idea. 
> 
> I see a another possibility in code - nesting init_block_directives() to
> plpgsql_ns_push and free_block_directives() to plpgsql_ns_pop()
> 

That's more or less what I mean by "integrating" to ns :)

The main idea is to not add 3rd layer of block push/pop that's sprinkled
in "random" places.

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

  15. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-03-20T10:30:56Z

    2017-03-19 14:30 GMT+01:00 Petr Jelinek <petr.jelinek@2ndquadrant.com>:

> On 19/03/17 12:32, Pavel Stehule wrote:
> >
> >
> > 2017-03-18 19:30 GMT+01:00 Petr Jelinek <petr.jelinek@2ndquadrant.com
> > <mailto:petr.jelinek@2ndquadrant.com>>:
> >
> >     On 16/03/17 17:15, David Steele wrote:
> >     > On 2/1/17 3:59 PM, Pavel Stehule wrote:
> >     >> Hi
> >     >>
> >     >> 2017-01-24 21:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com
> <mailto:pavel.stehule@gmail.com>
> >     >> <mailto:pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com
> >>>:
> >     >>
> >     >>             Perhaps that's as simple as renaming all the existing
> _ns_*
> >     >>             functions to _block_ and then adding support for
> pragmas...
> >     >>
> >     >>             Since you're adding cursor_options to PLpgSQL_expr it
> should
> >     >>             probably be removed as an option to exec_*.
> >     >>
> >     >>         I have to recheck it. Some cursor options going from
> dynamic
> >     >>         cursor variables and are related to dynamic query - not
> query
> >     >>         that creates query string.
> >     >>
> >     >>     hmm .. so current state is better due using options like
> >     >>     CURSOR_OPT_PARALLEL_OK
> >     >>
> >     >>          if (expr->plan == NULL)
> >     >>             exec_prepare_plan(estate, expr, (parallelOK ?
> >     >>                               CURSOR_OPT_PARALLEL_OK : 0) |
> >     >>     expr->cursor_options);
> >     >>
> >     >>     This options is not permanent feature of expression - and
> then I
> >     >>     cannot to remove cursor_option argument from exec_*
> >     >>
> >     >>     I did minor cleaning - remove cursor_options from plpgsql_var
> >     >>
> >     >> + basic doc
> >     >
> >     > This patch still applies cleanly and compiles at cccbdde.
> >     >
> >     > Any reviewers want to have a look?
> >     >
> >
> >     I'll bite.
> >
> >     I agree with Jim that it's not very nice to add yet another
> >     block/ns-like layer. I don't see why pragma could not be added to
> either
> >     PLpgSQL_stmt_block (yes pragma can be for whole function but function
> >     body is represented by PLpgSQL_stmt_block as well so no issue
> there), or
> >     to namespace code. In namespace since they are used for other thing
> >     there would be bit of unnecessary propagation but it's 8bytes per
> >     namespace, does not seem all that much.
> >
> >     My preference would be to add it to PLpgSQL_stmt_block (unless we
> plan
> >     to add posibility to add pragmas for other loops and other things)
> but I
> >     am not sure if current block is easily (and in a fast way) accessible
> >     from all places where it's needed. Maybe the needed info could be
> pushed
> >     to estate from PLpgSQL_stmt_block during the execution.
> >
> >
> > There is maybe partial misunderstand of pragma - it is set of nested
> > configurations used in compile time only. It can be used in execution
> > time too - it change nothing.
> >
> > The pragma doesn't build a persistent tree. It is stack of
> > configurations that allows fast access to current configuration, and
> > fast leaving of configuration when the change is out of scope.
> >
> > I don't see any any advantage to integrate pragma to ns or to
> > stmt_block. But maybe I don't understand to your idea.
> >
> > I see a another possibility in code - nesting init_block_directives() to
> > plpgsql_ns_push and free_block_directives() to plpgsql_ns_pop()
> >
>
> That's more or less what I mean by "integrating" to ns :)
>
> The main idea is to not add 3rd layer of block push/pop that's sprinkled
> in "random" places.
>

ok fixed

I reworked a maintaining settings - now it use lazy copy - the copy of
settings is created only when pragma is used in namespace. It remove any
impact on current code without pragmas.

Regards

Pavel


>
> --
>   Petr Jelinek                  http://www.2ndQuadrant.com/
>   PostgreSQL Development, 24x7 Support, Training & Services
>

  16. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-03-28T17:43:07Z

    Hi

rebased due last changes in pg_exec.c

Regards

Pavel

  17. Re: PoC plpgsql - possibility to force custom or generic plan

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2017-03-28T18:29:28Z

    On 28/03/17 19:43, Pavel Stehule wrote:
> Hi
> 
> rebased due last changes in pg_exec.c
> 

Thanks, I went over this and worked over the documentation/comments a
bit (attached updated version of the patch with my changes).

From my side this can go to committer.

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

  18. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-03-28T19:10:23Z

    2017-03-28 20:29 GMT+02:00 Petr Jelinek <petr.jelinek@2ndquadrant.com>:

> On 28/03/17 19:43, Pavel Stehule wrote:
> > Hi
> >
> > rebased due last changes in pg_exec.c
> >
>
> Thanks, I went over this and worked over the documentation/comments a
> bit (attached updated version of the patch with my changes).
>
> From my side this can go to committer.
>

Thank you

Pavel


>
> --
>   Petr Jelinek                  http://www.2ndQuadrant.com/
>   PostgreSQL Development, 24x7 Support, Training & Services
>

  19. Re: PoC plpgsql - possibility to force custom or generic plan

    Andres Freund <andres@anarazel.de> — 2017-04-05T20:33:32Z

    Hi,


I'd like some input from other committers whether we want this.  I'm
somewhat doubtful, but don't have particularly strong feelings.


> +
> +  <sect2 id="plpgsql-declaration-pragma">
> +   <title>Block level PRAGMA</title>
> +
> +   <indexterm>
> +    <primary>PRAGMA</>
> +    <secondary>in PL/pgSQL</>
> +   </indexterm>
> +
> +   <para>
> +    The block level <literal>PRAGMA</literal> allows to change the
> +    <application>PL/pgSQL</application> compiler behavior. Currently
> +    only <literal>PRAGMA PLAN_CACHE</literal> is supported.

Why are we doing this on a block level?


> +<programlisting>
> +CREATE FUNCTION enforce_fresh_plan(_id text) RETURNS boolean AS $$
> +DECLARE
> +  PRAGMA PLAN_CACHE(force_custom_plan);
> +BEGIN
> +  -- in this block every embedded query uses one shot plan

*plans


> +    <sect3 id="PRAGMA-PLAN_CACHE">
> +     <title>PRAGMA PLAN_CACHE</title>
> +
> +     <para>
> +      The plan cache behavior can be controlled using
> +      <literal>PRAGMA PLAN_CACHE</>. This <literal>PRAGMA</> can be used both
> +      for whole function or in individual blocks. The following options are

*functions


> +      possible: <literal>DEFAULT</literal> - default
> +      <application>PL/pgSQL</application> implementation - the system tries
> +      to decide between custom plan and generic plan after five query
> +      executions, <literal>FORCE_CUSTOM_PLAN</literal> - the chosen execution
> +      plan will be the one shot plan - it is specific for every set of
> +      used paramaters, <literal>FORCE_GENERIC_PLAN</literal> - the generic
> +      plan will be used from the start.

I don't think it's a good idea to explain this here, this'll just get
outdated.  I think we should rather have a link here.


> +     </para>
> +
> +     <para>
> +      <indexterm>
> +       <primary>PRAGMA PLAN_CACHE</>
> +       <secondary>in PL/pgSQL</>
> +      </indexterm>
> +      The plan for <command>INSERT</command> is always a generic
> plan.

That's this specific insert, right? Should be mentioned, sounds more
generic to me.

> +/* ----------
> + * Returns pointer to current compiler settings
> + * ----------
> + */
> +PLpgSQL_settings *
> +plpgsql_current_settings(void)
> +{
> +	return current_settings;
> +}
> +
> +
> +/* ----------
> + * Setup default compiler settings
> + * ----------
> + */
> +void
> +plpgsql_settings_init(PLpgSQL_settings *settings)
> +{
> +	current_settings = settings;
> +}

Hm. This is only ever set to &default_settings.


> +/* ----------
> + * Set compiler settings
> + * ----------
> + */
> +void
> +plpgsql_settings_set(PLpgSQL_settings *settings)
> +{
> +	PLpgSQL_nsitem *ns_cur = ns_top;
> +
> +	/*
> +	 * Modify settings directly, when ns has local settings data.
> +	 * When ns uses shared settings, create settings first.
> +	 */
> +	while (ns_cur->itemtype != PLPGSQL_NSTYPE_LABEL)
> +		ns_cur = ns_cur->prev;
> +
> +	if (ns_cur->local_settings == NULL)
> +	{
> +		ns_cur->local_settings = palloc(sizeof(PLpgSQL_settings));
> +		ns_cur->local_settings->prev = current_settings;
> +		current_settings = ns_cur->local_settings;
> +	}
> +
> +	current_settings->cursor_options = settings->cursor_options;
> +}

This seems like a somewhat weird method.  Why do we have a global
settings, when we essentially just want to use something in the current
ns?



- Andres

  20. Re: PoC plpgsql - possibility to force custom or generic plan

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-05T21:22:34Z

    Andres Freund <andres@anarazel.de> writes:
> I'd like some input from other committers whether we want this.  I'm
> somewhat doubtful, but don't have particularly strong feelings.

I don't really want to expose the workings of the plancache at user level.
The heuristics it uses certainly need work, but it'll get hard to change
those once there are SQL features depending on it.

Also, as you note, there are debatable design decisions in this particular
patch.  There are already a couple of ways in which control knobs can be
attached to plgsql functions (i.e. custom GUCs and the comp_option stuff),
so why is this patch wanting to invent yet another fundamental mechanism?
And I'm not very happy about it imposing a new reserved keyword, either.

A bigger-picture question is why we'd only provide such functionality
in plpgsql, and not for other uses of prepared plans.

Lastly, it doesn't look to me like the test cases prove anything at all
about whether the feature does what it's claimed to.

			regards, tom lane

  21. Re: PoC plpgsql - possibility to force custom or generic plan

    Andres Freund <andres@anarazel.de> — 2017-04-05T21:41:59Z

    On 2017-04-05 17:22:34 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I'd like some input from other committers whether we want this.  I'm
> > somewhat doubtful, but don't have particularly strong feelings.
> 
> I don't really want to expose the workings of the plancache at user level.
> The heuristics it uses certainly need work, but it'll get hard to change
> those once there are SQL features depending on it.
> 
> Also, as you note, there are debatable design decisions in this particular
> patch.  There are already a couple of ways in which control knobs can be
> attached to plgsql functions (i.e. custom GUCs and the comp_option stuff),
> so why is this patch wanting to invent yet another fundamental mechanism?
> And I'm not very happy about it imposing a new reserved keyword, either.
> 
> A bigger-picture question is why we'd only provide such functionality
> in plpgsql, and not for other uses of prepared plans.
> 
> Lastly, it doesn't look to me like the test cases prove anything at all
> about whether the feature does what it's claimed to.

That echoes my perception - so let's move this to the next CF?  It's not
like this patch has been pending for very long.

- Andres

  22. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-04-06T05:55:12Z

    2017-04-05 22:33 GMT+02:00 Andres Freund <andres@anarazel.de>:

> Hi,
>
>
> I'd like some input from other committers whether we want this.  I'm
> somewhat doubtful, but don't have particularly strong feelings.
>
>
> > +
> > +  <sect2 id="plpgsql-declaration-pragma">
> > +   <title>Block level PRAGMA</title>
> > +
> > +   <indexterm>
> > +    <primary>PRAGMA</>
> > +    <secondary>in PL/pgSQL</>
> > +   </indexterm>
> > +
> > +   <para>
> > +    The block level <literal>PRAGMA</literal> allows to change the
> > +    <application>PL/pgSQL</application> compiler behavior. Currently
> > +    only <literal>PRAGMA PLAN_CACHE</literal> is supported.
>
> Why are we doing this on a block level?
>

There are few reasons:

1. it is practical for some cases to mix more plan strategies in one
function

a)

FOR IN simple_select
LOOP
  ENFORCE ONE SHOT PLANS
  BEGIN
      .. queries ..
  END;
END LOOP;


b)

ENFORCE ONE SHOT PLANS
BEGIN
  FOR IN complex query requires one shot plan
  LOOP
    RETURNS TO DEFAULT PLAN CACHE
    BEGIN
      .. queries ..
    END;
  END LOOP;

2. This behave is defined in Ada language, and in PL/SQL too. If we will
have autonomous transactions, then we can have a equal functionality

a) run complete function under autonomous transaction
b) run some parts of function (some blocks) under autonomous transaction

It is not necessary, but it can avoid to generate auxiliary functions.


>
>
> > +<programlisting>
> > +CREATE FUNCTION enforce_fresh_plan(_id text) RETURNS boolean AS $$
> > +DECLARE
> > +  PRAGMA PLAN_CACHE(force_custom_plan);
> > +BEGIN
> > +  -- in this block every embedded query uses one shot plan
>
> *plans
>
>
> > +    <sect3 id="PRAGMA-PLAN_CACHE">
> > +     <title>PRAGMA PLAN_CACHE</title>
> > +
> > +     <para>
> > +      The plan cache behavior can be controlled using
> > +      <literal>PRAGMA PLAN_CACHE</>. This <literal>PRAGMA</> can be
> used both
> > +      for whole function or in individual blocks. The following options
> are
>
> *functions
>
>
> > +      possible: <literal>DEFAULT</literal> - default
> > +      <application>PL/pgSQL</application> implementation - the system
> tries
> > +      to decide between custom plan and generic plan after five query
> > +      executions, <literal>FORCE_CUSTOM_PLAN</literal> - the chosen
> execution
> > +      plan will be the one shot plan - it is specific for every set of
> > +      used paramaters, <literal>FORCE_GENERIC_PLAN</literal> - the
> generic
> > +      plan will be used from the start.
>
> I don't think it's a good idea to explain this here, this'll just get
> outdated.  I think we should rather have a link here.
>
>
> > +     </para>
> > +
> > +     <para>
> > +      <indexterm>
> > +       <primary>PRAGMA PLAN_CACHE</>
> > +       <secondary>in PL/pgSQL</>
> > +      </indexterm>
> > +      The plan for <command>INSERT</command> is always a generic
> > plan.
>
> That's this specific insert, right? Should be mentioned, sounds more
> generic to me.
>
> > +/* ----------
> > + * Returns pointer to current compiler settings
> > + * ----------
> > + */
> > +PLpgSQL_settings *
> > +plpgsql_current_settings(void)
> > +{
> > +     return current_settings;
> > +}
> > +
> > +
> > +/* ----------
> > + * Setup default compiler settings
> > + * ----------
> > + */
> > +void
> > +plpgsql_settings_init(PLpgSQL_settings *settings)
> > +{
> > +     current_settings = settings;
> > +}
>
> Hm. This is only ever set to &default_settings.
>
>
> > +/* ----------
> > + * Set compiler settings
> > + * ----------
> > + */
> > +void
> > +plpgsql_settings_set(PLpgSQL_settings *settings)
> > +{
> > +     PLpgSQL_nsitem *ns_cur = ns_top;
> > +
> > +     /*
> > +      * Modify settings directly, when ns has local settings data.
> > +      * When ns uses shared settings, create settings first.
> > +      */
> > +     while (ns_cur->itemtype != PLPGSQL_NSTYPE_LABEL)
> > +             ns_cur = ns_cur->prev;
> > +
> > +     if (ns_cur->local_settings == NULL)
> > +     {
> > +             ns_cur->local_settings = palloc(sizeof(PLpgSQL_settings));
> > +             ns_cur->local_settings->prev = current_settings;
> > +             current_settings = ns_cur->local_settings;
> > +     }
> > +
> > +     current_settings->cursor_options = settings->cursor_options;
> > +}
>
> This seems like a somewhat weird method.  Why do we have a global
> settings, when we essentially just want to use something in the current
> ns?
>
>
I am not sure if I understand to question.

This settings is implemented as lazy. If ns has not any own settings, then
nothing is done. It requires some global variable, because some ns can be
skipped.

My first implementation was 1:1 .. ns:settings - but it add some overhead
for any ns although ns has not own settings.

Regards

Pavel



>
>
> - Andres
>

  23. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-04-06T06:08:13Z

    2017-04-05 23:22 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

> Andres Freund <andres@anarazel.de> writes:
> > I'd like some input from other committers whether we want this.  I'm
> > somewhat doubtful, but don't have particularly strong feelings.
>
> I don't really want to expose the workings of the plancache at user level.
> The heuristics it uses certainly need work, but it'll get hard to change
> those once there are SQL features depending on it.
>

I am very sceptical about enhancing heuristics - but I am open to any
proposals.

The advanced users disable a plan cache with dynamic SQL. But this
workaround has strong disadvantages:

1. it is vulnerable to SQL injection
2. it is less readable



>
> Also, as you note, there are debatable design decisions in this particular
> patch.  There are already a couple of ways in which control knobs can be
> attached to plgsql functions (i.e. custom GUCs and the comp_option stuff),
> so why is this patch wanting to invent yet another fundamental mechanism?
> And I'm not very happy about it imposing a new reserved keyword, either.
>

1.

custom GUC has not local scope - so it doesn't allow precious settings.
With PRAGMA I have perfect control what will be impacted.

#option has function scope

2. I'll not introduce a PRAGMA keyword just for this feature. We would to
implement autonomous transactions. There was not any objection against this
feature. The PRAGMA allows to share PL/SQL syntax and functionality.


> A bigger-picture question is why we'd only provide such functionality
> in plpgsql, and not for other uses of prepared plans.
>

It is out of scope of this patch.


>
> Lastly, it doesn't look to me like the test cases prove anything at all
> about whether the feature does what it's claimed to.
>

I can enhance regress tests - currently there are not direct access to
these attributes - so the tests can be indirect only :(

Regards

Pavel


>
>                         regards, tom lane
>

  24. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-04-06T06:09:29Z

    >
> That echoes my perception - so let's move this to the next CF?  It's not
> like this patch has been pending for very long.
>

sure

Regards

Pavel


>
> - Andres
>

  25. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-04-06T06:17:12Z

    2017-04-06 8:08 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

>
>
> 2017-04-05 23:22 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
>
>> Andres Freund <andres@anarazel.de> writes:
>> > I'd like some input from other committers whether we want this.  I'm
>> > somewhat doubtful, but don't have particularly strong feelings.
>>
>> I don't really want to expose the workings of the plancache at user level.
>> The heuristics it uses certainly need work, but it'll get hard to change
>> those once there are SQL features depending on it.
>>
>
> I am very sceptical about enhancing heuristics - but I am open to any
> proposals.
>
> The advanced users disable a plan cache with dynamic SQL. But this
> workaround has strong disadvantages:
>
> 1. it is vulnerable to SQL injection
> 2. it is less readable
>
>
>
>>
>> Also, as you note, there are debatable design decisions in this particular
>> patch.  There are already a couple of ways in which control knobs can be
>> attached to plgsql functions (i.e. custom GUCs and the comp_option stuff),
>> so why is this patch wanting to invent yet another fundamental mechanism?
>> And I'm not very happy about it imposing a new reserved keyword, either.
>>
>
> 1.
>
> custom GUC has not local scope - so it doesn't allow precious settings.
> With PRAGMA I have perfect control what will be impacted.
>
> #option has function scope
>
> 2. I'll not introduce a PRAGMA keyword just for this feature. We would to
> implement autonomous transactions. There was not any objection against this
> feature. The PRAGMA allows to share PL/SQL syntax and functionality.
>
>
>> A bigger-picture question is why we'd only provide such functionality
>> in plpgsql, and not for other uses of prepared plans.
>>
>
> It is out of scope of this patch.
>

The scope of this patch can be enhanced - but it is different task because
user interface should be different.


>
>
>>
>> Lastly, it doesn't look to me like the test cases prove anything at all
>> about whether the feature does what it's claimed to.
>>
>
> I can enhance regress tests - currently there are not direct access to
> these attributes - so the tests can be indirect only :(
>
> Regards
>
> Pavel
>
>
>>
>>                         regards, tom lane
>>
>
>

  26. Re: PoC plpgsql - possibility to force custom or generic plan

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2017-04-06T07:04:06Z

    On 05/04/17 23:22, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> I'd like some input from other committers whether we want this.  I'm
>> somewhat doubtful, but don't have particularly strong feelings.
> 
> I don't really want to expose the workings of the plancache at user level.
> The heuristics it uses certainly need work, 

That's an understatement, there are thousands of plpgsql functions in
large installations of PostgreSQL which use EXECUTE everywhere just to
avoid current behavior (and that's just what I've seen).

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

  27. Re: PoC plpgsql - possibility to force custom or generic plan

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2017-04-06T10:30:07Z

    
On 04/05/2017 05:41 PM, Andres Freund wrote:
> On 2017-04-05 17:22:34 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> I'd like some input from other committers whether we want this.  I'm
>>> somewhat doubtful, but don't have particularly strong feelings.
>> I don't really want to expose the workings of the plancache at user level.
>> The heuristics it uses certainly need work, but it'll get hard to change
>> those once there are SQL features depending on it.
>>
>> Also, as you note, there are debatable design decisions in this particular
>> patch.  There are already a couple of ways in which control knobs can be
>> attached to plgsql functions (i.e. custom GUCs and the comp_option stuff),
>> so why is this patch wanting to invent yet another fundamental mechanism?
>> And I'm not very happy about it imposing a new reserved keyword, either.
>>
>> A bigger-picture question is why we'd only provide such functionality
>> in plpgsql, and not for other uses of prepared plans.
>>
>> Lastly, it doesn't look to me like the test cases prove anything at all
>> about whether the feature does what it's claimed to.
> That echoes my perception - so let's move this to the next CF?  It's not
> like this patch has been pending for very long.
>


Or just Return with Feedback.

ISTM before we revisit this we need agreement on a design.

cheers

andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  28. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-04-06T18:32:35Z

    2017-04-06 12:30 GMT+02:00 Andrew Dunstan <andrew.dunstan@2ndquadrant.com>:

>
>
> On 04/05/2017 05:41 PM, Andres Freund wrote:
> > On 2017-04-05 17:22:34 -0400, Tom Lane wrote:
> >> Andres Freund <andres@anarazel.de> writes:
> >>> I'd like some input from other committers whether we want this.  I'm
> >>> somewhat doubtful, but don't have particularly strong feelings.
> >> I don't really want to expose the workings of the plancache at user
> level.
> >> The heuristics it uses certainly need work, but it'll get hard to change
> >> those once there are SQL features depending on it.
> >>
> >> Also, as you note, there are debatable design decisions in this
> particular
> >> patch.  There are already a couple of ways in which control knobs can be
> >> attached to plgsql functions (i.e. custom GUCs and the comp_option
> stuff),
> >> so why is this patch wanting to invent yet another fundamental
> mechanism?
> >> And I'm not very happy about it imposing a new reserved keyword, either.
> >>
> >> A bigger-picture question is why we'd only provide such functionality
> >> in plpgsql, and not for other uses of prepared plans.
> >>
> >> Lastly, it doesn't look to me like the test cases prove anything at all
> >> about whether the feature does what it's claimed to.
> > That echoes my perception - so let's move this to the next CF?  It's not
> > like this patch has been pending for very long.
> >
>
>
> Or just Return with Feedback.
>
> ISTM before we revisit this we need agreement on a design.
>

I am open to any ideas - there are some my start points

1. the possibility to disable plan cache is real request
2. can be useful if we are able to control plan cache inside function - the
mix of settings is real case too
3. GUC are useless - nobody would to disable plan cache globally

I like to see any proposals about syntax or implementation.

Using PRAGMA is one variant - introduced by PLpgSQL origin - Ada language.
The PRAGMA syntax can be used for PRAGMA autonomous with well known syntax.
It scales well  - it supports function, block or command level.

I invite any discussion now or in start of new release cycle.

Regards

Pavel





> cheers
>
> andrew
>
>
> --
> Andrew Dunstan                https://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

  29. Re: PoC plpgsql - possibility to force custom or generic plan

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-04-08T00:30:59Z

    On 4/6/17 14:32, Pavel Stehule wrote:
> I like to see any proposals about syntax or implementation.
> 
> Using PRAGMA is one variant - introduced by PLpgSQL origin - Ada
> language. The PRAGMA syntax can be used for PRAGMA autonomous with well
> known syntax. It scales well  - it supports function, block or command
> level.

I had pragmas implemented in the original autonomous transactions patch
(https://www.postgresql.org/message-id/659a2fce-b6ee-06de-05c0-c8ed6a01979e@2ndquadrant.com).
 However, the difference there is that the behavior is lexical, specific
to plpgsql, whereas here you are really just selecting run time
behavior.  So a GUC, and also something that could apply to other
places, should be considered.

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

  30. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-04-08T07:42:44Z

    2017-04-08 2:30 GMT+02:00 Peter Eisentraut <peter.eisentraut@2ndquadrant.com
>:

> On 4/6/17 14:32, Pavel Stehule wrote:
> > I like to see any proposals about syntax or implementation.
> >
> > Using PRAGMA is one variant - introduced by PLpgSQL origin - Ada
> > language. The PRAGMA syntax can be used for PRAGMA autonomous with well
> > known syntax. It scales well  - it supports function, block or command
> > level.
>
> I had pragmas implemented in the original autonomous transactions patch
> (https://www.postgresql.org/message-id/659a2fce-b6ee-06de-
> 05c0-c8ed6a01979e@2ndquadrant.com).
>  However, the difference there is that the behavior is lexical, specific
> to plpgsql, whereas here you are really just selecting run time
> behavior.  So a GUC, and also something that could apply to other
> places, should be considered.
>

I'll look there - we coordinate work on that.

Regards

Pavel


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

  31. Re: PoC plpgsql - possibility to force custom or generic plan

    Daniel Gustafsson <daniel@yesql.se> — 2017-09-05T13:01:54Z

    > On 08 Apr 2017, at 09:42, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 
> 2017-04-08 2:30 GMT+02:00 Peter Eisentraut <peter.eisentraut@2ndquadrant.com <mailto:peter.eisentraut@2ndquadrant.com>>:
> On 4/6/17 14:32, Pavel Stehule wrote:
> > I like to see any proposals about syntax or implementation.
> >
> > Using PRAGMA is one variant - introduced by PLpgSQL origin - Ada
> > language. The PRAGMA syntax can be used for PRAGMA autonomous with well
> > known syntax. It scales well  - it supports function, block or command
> > level.
> 
> I had pragmas implemented in the original autonomous transactions patch
> (https://www.postgresql.org/message-id/659a2fce-b6ee-06de-05c0-c8ed6a01979e@2ndquadrant.com <https://www.postgresql.org/message-id/659a2fce-b6ee-06de-05c0-c8ed6a01979e@2ndquadrant.com>).
>  However, the difference there is that the behavior is lexical, specific
> to plpgsql, whereas here you are really just selecting run time
> behavior.  So a GUC, and also something that could apply to other
> places, should be considered.
> 
> I'll look there - we coordinate work on that.

This patch was moved to the now started commitfest, and is marked as “Needs
review”.  Based on this thread I will however change it to "waiting for author”,
since there seems to be some open questions.  Has there been any new work done
on this towards a new design/patch?

cheers ./daniel

  32. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-05T17:18:42Z

    2017-09-05 15:01 GMT+02:00 Daniel Gustafsson <daniel@yesql.se>:

> > On 08 Apr 2017, at 09:42, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> >
> > 2017-04-08 2:30 GMT+02:00 Peter Eisentraut <
> peter.eisentraut@2ndquadrant.com <mailto:peter.eisentraut@2ndquadrant.com
> >>:
> > On 4/6/17 14:32, Pavel Stehule wrote:
> > > I like to see any proposals about syntax or implementation.
> > >
> > > Using PRAGMA is one variant - introduced by PLpgSQL origin - Ada
> > > language. The PRAGMA syntax can be used for PRAGMA autonomous with well
> > > known syntax. It scales well  - it supports function, block or command
> > > level.
> >
> > I had pragmas implemented in the original autonomous transactions patch
> > (https://www.postgresql.org/message-id/659a2fce-b6ee-06de-
> 05c0-c8ed6a01979e@2ndquadrant.com <https://www.postgresql.org/
> message-id/659a2fce-b6ee-06de-05c0-c8ed6a01979e@2ndquadrant.com>).
> >  However, the difference there is that the behavior is lexical, specific
> > to plpgsql, whereas here you are really just selecting run time
> > behavior.  So a GUC, and also something that could apply to other
> > places, should be considered.
> >
> > I'll look there - we coordinate work on that.
>
> This patch was moved to the now started commitfest, and is marked as “Needs
> review”.  Based on this thread I will however change it to "waiting for
> author”,
> since there seems to be some open questions.  Has there been any new work
> done
> on this towards a new design/patch?
>

I didn't any work on this patch last months. I hope so this week I reread
this thread and I'll check what I do.

There are few but important questions:

1. we want this feature? I hope so we want - because I don't believe to
user invisible great solution - and this is simple solution that helps with
readability of some complex PL procedures.

2. what syntax we should to use (if we accept this feature)? There was not
another proposal if I remember well - The PRAGMA syntax is strong because
we can very well specify to range where the plans caching will be
explicitly controlled. It is well readable and static.

Regards

Pavel

>
> cheers ./daniel

  33. Re: PoC plpgsql - possibility to force custom or generic plan

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-05T17:38:42Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2. what syntax we should to use (if we accept this feature)? There was not
> another proposal if I remember well - The PRAGMA syntax is strong because
> we can very well specify to range where the plans caching will be
> explicitly controlled. It is well readable and static.

The complaint I have about PRAGMA is that it's yet another syntax for
accomplishing pretty much the same thing.  If you don't like the GUC
solution, we've already got the "comp_option" syntax for static options
in plpgsql.  Sure, that's not too pretty, but that's not a good reason
to invent yet another way to do it.

			regards, tom lane

  34. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-05T17:51:51Z

    2017-09-05 19:38 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > 2. what syntax we should to use (if we accept this feature)? There was
> not
> > another proposal if I remember well - The PRAGMA syntax is strong because
> > we can very well specify to range where the plans caching will be
> > explicitly controlled. It is well readable and static.
>
> The complaint I have about PRAGMA is that it's yet another syntax for
> accomplishing pretty much the same thing.  If you don't like the GUC
> solution, we've already got the "comp_option" syntax for static options
> in plpgsql.  Sure, that's not too pretty, but that's not a good reason
> to invent yet another way to do it.
>

comp_option has only function scope, what is too limited for this purpose.

I don't prefer GUC for this purpose because you need to do SET/RESET on two
places. With GUC the code can looks like:

PERFORM set_config('cachexx', 'off')
FOR r IN SELECT ...
LOOP
  PERFORM set_config(' cachexx', 'on')
  ....
  PERFORM set_config('cachexx', 'off')
END LOOP;
PERFORM set_config('cachexx', 'on');


The another reason for inventing PRAGMA syntax to PLpgSQL was support for
autonomous transaction and I am thinking so is good idea using same syntax
like PL/SQL does.


>
>                         regards, tom lane
>

  35. Re: PoC plpgsql - possibility to force custom or generic plan

    Robert Haas <robertmhaas@gmail.com> — 2017-09-06T14:43:39Z

    On Tue, Sep 5, 2017 at 1:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The complaint I have about PRAGMA is that it's yet another syntax for
> accomplishing pretty much the same thing.  If you don't like the GUC
> solution, we've already got the "comp_option" syntax for static options
> in plpgsql.  Sure, that's not too pretty, but that's not a good reason
> to invent yet another way to do it.

On the general question of whether we should have something like this,
I expressed a lot of doubt when
e6faf910d75027bdce7cd0f2033db4e912592bcc first went in about whether
that algorithm was really going to work, and nothing has happened
since then to remove any of that doubt.  It's pretty clear to me from
experiences with customer problems that the heuristics we have often
fail to do the right thing, either because queries with no hope of
benefiting still replan 5 times - which can waste a ton of CPU when
there are many different queries in the plan cache and many sessions -
or because queries that would benefit in some cases give up on
replanning before they hit a case where a parameter-specific plan
helps.  I don't think we can just indefinitely continue to resist
providing manual control over this behavior on the theory that some
day we'll fix it.  It's been six years and we haven't made any
significant progress.  In some cases, a long delay without any
progress might just point to a lack of effort that should have been
applied, but in this case I think it's because the problem is
incredibly hard.

I think what we ideally want to do is notice whether the new bind
variables cause a change in selectivity which is sufficient to justify
a re-plan.  If we annotated the original plan with markers indicating
that it was valid for all values with a frequency of more than X and
less than Y, for example, we could cover most cases involving
equality; range queries would need some other kind of annotation.
However, it's unclear how the planner could produce such annotations,
and it's unclear how expensive checking against them would be if we
had them.  Barring somebody having a brilliant insight about how to
make some system that's way better than what we have right now, I
think we can't hold out much hope of any better fix than a manual
knob.

I think a GUC is a decent, though not perfect, mechanism for this.
This problem isn't restricted to PL/pgsql; indeed, the cases I've seen
have come via prepared queries, not PL/pgsql functions.  Even without
that, one advantage of a GUC is that they are fairly broadly
understood and experienced users understand what they can do with
them.  They can be set at various different scopes (system, user,
database, SET clause for a particular function) and it's relatively
convenient to do so.  Some kind of PL/pgsql-specific PRAGMA syntax is
more likely to be overlooked by users who would actually benefit from
it, and also won't cover non-PL/pgsql cases.  If we were going to go
the PRAGMA route, it would make more sense to me to define that as a
way of setting a GUC for the scope of one PL/pgsql block, like PRAGMA
SETTING(custom_plan_tries, 0).  I think it is in general unfortunate
that we don't have a mechanism to change a GUC for the lifespan of one
particular query, like this:

LET custom_plan_tries = 0 IN SELECT ...

I bet a lot of people would find that quite convenient.  The problem
of needing to set a planner GUC for one particular query is pretty
common, and Pavel is absolutely right that having to do SET beforehand
and RESET afterward is ugly.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  36. Re: PoC plpgsql - possibility to force custom or generic plan

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-06T15:03:17Z

    Robert Haas <robertmhaas@gmail.com> writes:
> I don't think we can just indefinitely continue to resist
> providing manual control over this behavior on the theory that some
> day we'll fix it.

That's fair enough.  We need to have a discussion about exactly what
the knob does, which is distinct from the question of how you spell
the incantation for twiddling it.  I'm dubious that a dumb "force a
custom plan" setting is going to solve all that many cases usefully.

> I think a GUC is a decent, though not perfect, mechanism for this.
> This problem isn't restricted to PL/pgsql; indeed, the cases I've seen
> have come via prepared queries, not PL/pgsql functions.

That's 100% correct, and is actually the best reason not to consider
a PRAGMA (or any other plpgsql-specific mechanism) as the incantation
spelling.

> I think it is in general unfortunate that we don't have a mechanism to
> change a GUC for the lifespan of one particular query, like this:

> LET custom_plan_tries = 0 IN SELECT ...

Hmm.  I think the core problem here is that we're trying to control
the plancache, which is a pretty much behind-the-scenes mechanism.
Except in the case of an explicit PREPARE, you can't even see from
SQL that the cache is being used, or when it's used.  So part of what
needs to be thought about, if we use the GUC approach, is when the
GUC's value is consulted.  If we don't do anything special then
the GUC(s) would be consulted when retrieving plans from the cache,
and changes in their values from one retrieval to the next might
cause funny behavior.  Maybe the relevant settings need to be captured
when the plancache entry is made ... not sure.

			regards, tom lane

  37. Re: PoC plpgsql - possibility to force custom or generic plan

    Robert Haas <robertmhaas@gmail.com> — 2017-09-06T15:17:23Z

    On Wed, Sep 6, 2017 at 11:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> That's fair enough.  We need to have a discussion about exactly what
> the knob does, which is distinct from the question of how you spell
> the incantation for twiddling it.  I'm dubious that a dumb "force a
> custom plan" setting is going to solve all that many cases usefully.

I think what people need is the ability to force the behavior in
either direction - either insist on a custom plan, or insist on a
generic plan.  The former is what you need if the plan stinks, and the
latter is what you need if replanning is a waste of effort.  I have
seen both cases.  The latter has been a bigger problem than the
former, because the former can be hacked around in various ugly and
inefficient ways, but if we're adding a knob I think it should have
three settings.  There is perhaps an argument for even more
configurability, like altering the number of plan tries from 5 to some
other value, but I'm not clear that there's any use case for values
other than 0, 5, and +infinity.

>> I think it is in general unfortunate that we don't have a mechanism to
>> change a GUC for the lifespan of one particular query, like this:
>
>> LET custom_plan_tries = 0 IN SELECT ...
>
> Hmm.  I think the core problem here is that we're trying to control
> the plancache, which is a pretty much behind-the-scenes mechanism.
> Except in the case of an explicit PREPARE, you can't even see from
> SQL that the cache is being used, or when it's used.  So part of what
> needs to be thought about, if we use the GUC approach, is when the
> GUC's value is consulted.  If we don't do anything special then
> the GUC(s) would be consulted when retrieving plans from the cache,
> and changes in their values from one retrieval to the next might
> cause funny behavior.  Maybe the relevant settings need to be captured
> when the plancache entry is made ... not sure.

What sort of funny behavior are you concerned about?  It seems likely
to me that in most cases the GUC will have the same value every time
through, but if it doesn't, I'm not sure why we'd need to use the old
value rather than the current one.  Indeed, if the user changes the
GUC from "force custom" to "force generic" and reruns the function, we
want the new value to take effect, lest a POLA violation occur.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  38. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-06T18:45:25Z

    >
>
> I think a GUC is a decent, though not perfect, mechanism for this.
> This problem isn't restricted to PL/pgsql; indeed, the cases I've seen
> have come via prepared queries, not PL/pgsql functions.  Even without
> that, one advantage of a GUC is that they are fairly broadly
> understood and experienced users understand what they can do with
> them.  They can be set at various different scopes (system, user,
> database, SET clause for a particular function) and it's relatively
> convenient to do so.  Some kind of PL/pgsql-specific PRAGMA syntax is
> more likely to be overlooked by users who would actually benefit from
> it, and also won't cover non-PL/pgsql cases.  If we were going to go
> the PRAGMA route, it would make more sense to me to define that as a
> way of setting a GUC for the scope of one PL/pgsql block, like PRAGMA
> SETTING(custom_plan_tries, 0).  I think it is in general unfortunate
> that we don't have a mechanism to change a GUC for the lifespan of one
> particular query, like this:
>
> LET custom_plan_tries = 0 IN SELECT ...
>
> I bet a lot of people would find that quite convenient.  The problem
> of needing to set a planner GUC for one particular query is pretty
> common, and Pavel is absolutely right that having to do SET beforehand
> and RESET afterward is ugly.
>

Currently only prepared statement and PLpgSQL uses plan cache, what I know
- so some special GUC has sense only for this two environments.

I understand so GUC can be used and has sense when users cannot to modify
source code or when they want to apply this change globally.

For PLpgSQL there should be block, or statement level related syntax -
PRAGMA is well known alternative from ADA language. Maybe another syntax
like

BEGIN SET xxx = 1, yyy = 2 .. END ... theoretically we can introduce block
level GUC. I don't like it, because there are successful syntax from ADA,
PL/SQL.

Maybe can be useful enhance a PREPARE command to accept second optional
parameter for plan cache controlling - I have not idea about syntax.

Regards

Pavel


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

  39. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-07T19:30:22Z

    > >
> > Hmm.  I think the core problem here is that we're trying to control
> > the plancache, which is a pretty much behind-the-scenes mechanism.
> > Except in the case of an explicit PREPARE, you can't even see from
> > SQL that the cache is being used, or when it's used.  So part of what
> > needs to be thought about, if we use the GUC approach, is when the
> > GUC's value is consulted.  If we don't do anything special then
> > the GUC(s) would be consulted when retrieving plans from the cache,
> > and changes in their values from one retrieval to the next might
> > cause funny behavior.  Maybe the relevant settings need to be captured
> > when the plancache entry is made ... not sure.
>
> What sort of funny behavior are you concerned about?  It seems likely
> to me that in most cases the GUC will have the same value every time
> through, but if it doesn't, I'm not sure why we'd need to use the old
> value rather than the current one.  Indeed, if the user changes the
> GUC from "force custom" to "force generic" and reruns the function, we
> want the new value to take effect, lest a POLA violation occur.
>

good note - so changing this GUC on session level requires reset plan cache.

I am not against to GUC, and I am not against to PLpgSQL #option. Just, and
I am repeating (I am sorry) - these tools are not practical for usage in
PLpgSQL. There should be some block level possibility to do some setting.

Regards

Pavel


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

  40. Re: PoC plpgsql - possibility to force custom or generic plan

    Simon Riggs <simon@2ndquadrant.com> — 2017-09-08T17:14:50Z

    On 6 September 2017 at 07:43, Robert Haas <robertmhaas@gmail.com> wrote:

> LET custom_plan_tries = 0 IN SELECT ...

Tom has pointed me at this proposal, since on another thread I asked
for something very similar. (No need to reprise that discussion, but I
wanted prepared queries to be able to do SET work_mem = X; SELECT).
This idea looks a good way forward to me.

Since we're all in roughly the same place, I'd like to propose that we
proceed with the following syntax... whether or not this precisely
solves OP's issue on this thread.

1. Allow SET to set multiple parameters...
SET guc1 = x, guc2 = y
This looks fairly straightforward

2. Allow a SET to apply only for a single statement
SET guc1 = x, guc2 = y FOR stmt
e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
Internally a GUC setting already exists for a single use, via
GUC_ACTION_SAVE, so we just need to invoke it.

Quick prototype seems like it will deliver quite quickly. I couldn't
see a reason to use "LET" rather than just "SET" which would be the
POLA choice.

Thoughts?

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

  41. Re: PoC plpgsql - possibility to force custom or generic plan

    Simon Riggs <simon@2ndquadrant.com> — 2017-09-08T17:14:50Z

    On 6 September 2017 at 07:43, Robert Haas <robertmhaas@gmail.com> wrote:

> LET custom_plan_tries = 0 IN SELECT ...

Tom has pointed me at this proposal, since on another thread I asked
for something very similar. (No need to reprise that discussion, but I
wanted prepared queries to be able to do SET work_mem = X; SELECT).
This idea looks a good way forward to me.

Since we're all in roughly the same place, I'd like to propose that we
proceed with the following syntax... whether or not this precisely
solves OP's issue on this thread.

1. Allow SET to set multiple parameters...
SET guc1 = x, guc2 = y
This looks fairly straightforward

2. Allow a SET to apply only for a single statement
SET guc1 = x, guc2 = y FOR stmt
e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
Internally a GUC setting already exists for a single use, via
GUC_ACTION_SAVE, so we just need to invoke it.

Quick prototype seems like it will deliver quite quickly. I couldn't
see a reason to use "LET" rather than just "SET" which would be the
POLA choice.

Thoughts?

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

  42. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-08T17:31:33Z

    2017-09-08 19:14 GMT+02:00 Simon Riggs <simon@2ndquadrant.com>:

> On 6 September 2017 at 07:43, Robert Haas <robertmhaas@gmail.com> wrote:
>
> > LET custom_plan_tries = 0 IN SELECT ...
>
> Tom has pointed me at this proposal, since on another thread I asked
> for something very similar. (No need to reprise that discussion, but I
> wanted prepared queries to be able to do SET work_mem = X; SELECT).
> This idea looks a good way forward to me.
>
> Since we're all in roughly the same place, I'd like to propose that we
> proceed with the following syntax... whether or not this precisely
> solves OP's issue on this thread.
>
> 1. Allow SET to set multiple parameters...
> SET guc1 = x, guc2 = y
> This looks fairly straightforward
>
> 2. Allow a SET to apply only for a single statement
> SET guc1 = x, guc2 = y FOR stmt
> e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
> Internally a GUC setting already exists for a single use, via
> GUC_ACTION_SAVE, so we just need to invoke it.
>
> Quick prototype seems like it will deliver quite quickly. I couldn't
> see a reason to use "LET" rather than just "SET" which would be the
> POLA choice.
>
> Thoughts?
>

why not

Pavel


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

  43. Re: PoC plpgsql - possibility to force custom or generic plan

    Daniel Gustafsson <daniel@yesql.se> — 2017-09-08T19:21:52Z

    > On 08 Sep 2017, at 19:14, Simon Riggs <simon@2ndquadrant.com> wrote:
> 
> On 6 September 2017 at 07:43, Robert Haas <robertmhaas@gmail.com> wrote:
> 
>> LET custom_plan_tries = 0 IN SELECT ...
> 
> Tom has pointed me at this proposal, since on another thread I asked
> for something very similar. (No need to reprise that discussion, but I
> wanted prepared queries to be able to do SET work_mem = X; SELECT).
> This idea looks a good way forward to me.
> 
> Since we're all in roughly the same place, I'd like to propose that we
> proceed with the following syntax... whether or not this precisely
> solves OP's issue on this thread.
> 
> 1. Allow SET to set multiple parameters...
> SET guc1 = x, guc2 = y
> This looks fairly straightforward
> 
> 2. Allow a SET to apply only for a single statement
> SET guc1 = x, guc2 = y FOR stmt
> e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
> Internally a GUC setting already exists for a single use, via
> GUC_ACTION_SAVE, so we just need to invoke it.

This syntax proposal makes sense, +1.  My immediate thought was that the
per-statement GUCs were sort of like options, and most options in our syntax
are enclosed with (), like: SET (guc1 = x, guc2 = y) FOR SELECT ..;

cheers ./daniel

  44. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-08T19:48:33Z

    2017-09-08 21:21 GMT+02:00 Daniel Gustafsson <daniel@yesql.se>:

> > On 08 Sep 2017, at 19:14, Simon Riggs <simon@2ndquadrant.com> wrote:
> >
> > On 6 September 2017 at 07:43, Robert Haas <robertmhaas@gmail.com> wrote:
> >
> >> LET custom_plan_tries = 0 IN SELECT ...
> >
> > Tom has pointed me at this proposal, since on another thread I asked
> > for something very similar. (No need to reprise that discussion, but I
> > wanted prepared queries to be able to do SET work_mem = X; SELECT).
> > This idea looks a good way forward to me.
> >
> > Since we're all in roughly the same place, I'd like to propose that we
> > proceed with the following syntax... whether or not this precisely
> > solves OP's issue on this thread.
> >
> > 1. Allow SET to set multiple parameters...
> > SET guc1 = x, guc2 = y
> > This looks fairly straightforward
> >
> > 2. Allow a SET to apply only for a single statement
> > SET guc1 = x, guc2 = y FOR stmt
> > e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
> > Internally a GUC setting already exists for a single use, via
> > GUC_ACTION_SAVE, so we just need to invoke it.
>
> This syntax proposal makes sense, +1.  My immediate thought was that the
> per-statement GUCs were sort of like options, and most options in our
> syntax
> are enclosed with (), like: SET (guc1 = x, guc2 = y) FOR SELECT ..;
>

we newer support this syntax in combination with SET keyword

see - CREATE FUNCTION command

personally I prefer syntax without FOR keyword - because following keyword
must be reserved keyword

SET x = .., y = .. SELECT ... ;

Regards

Pavel


> cheers ./daniel
>

  45. Re: PoC plpgsql - possibility to force custom or generic plan

    Merlin Moncure <mmoncure@gmail.com> — 2017-09-08T20:38:17Z

    On Fri, Sep 8, 2017 at 2:48 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
> 2017-09-08 21:21 GMT+02:00 Daniel Gustafsson <daniel@yesql.se>:
>>
>> > On 08 Sep 2017, at 19:14, Simon Riggs <simon@2ndquadrant.com> wrote:
>> >
>> > On 6 September 2017 at 07:43, Robert Haas <robertmhaas@gmail.com> wrote:
>> >
>> >> LET custom_plan_tries = 0 IN SELECT ...
>> >
>> > Tom has pointed me at this proposal, since on another thread I asked
>> > for something very similar. (No need to reprise that discussion, but I
>> > wanted prepared queries to be able to do SET work_mem = X; SELECT).
>> > This idea looks a good way forward to me.
>> >
>> > Since we're all in roughly the same place, I'd like to propose that we
>> > proceed with the following syntax... whether or not this precisely
>> > solves OP's issue on this thread.
>> >
>> > 1. Allow SET to set multiple parameters...
>> > SET guc1 = x, guc2 = y
>> > This looks fairly straightforward
>> >
>> > 2. Allow a SET to apply only for a single statement
>> > SET guc1 = x, guc2 = y FOR stmt
>> > e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
>> > Internally a GUC setting already exists for a single use, via
>> > GUC_ACTION_SAVE, so we just need to invoke it.
>>
>> This syntax proposal makes sense, +1.  My immediate thought was that the
>> per-statement GUCs were sort of like options, and most options in our
>> syntax
>> are enclosed with (), like: SET (guc1 = x, guc2 = y) FOR SELECT ..;
>
> we newer support this syntax in combination with SET keyword
>
> see - CREATE FUNCTION command
>
> personally I prefer syntax without FOR keyword - because following keyword
> must be reserved keyword
>
> SET x = .., y = .. SELECT ... ;

This seems pretty ugly from a syntax perspective.

We already have 'SET LOCAL', which manages scope to the current
transaction.  How about SET BLOCK which would set until you've left
the current statement block?

merlin

  46. Re: PoC plpgsql - possibility to force custom or generic plan

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-08T21:09:37Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> personally I prefer syntax without FOR keyword - because following keyword
> must be reserved keyword

> SET x = .., y = .. SELECT ... ;

Nope.  Most of the statement-starting keywords are *not* fully reserved;
they don't need to be as long as they lead off the statement.  But this
proposal would break that.  We need to put FOR or IN or another
already-fully-reserved keyword after the SET list, or something's going
to bite us.

			regards, tom lane

  47. Re: PoC plpgsql - possibility to force custom or generic plan

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-08T21:14:15Z

    Merlin Moncure <mmoncure@gmail.com> writes:
> We already have 'SET LOCAL', which manages scope to the current
> transaction.  How about SET BLOCK which would set until you've left
> the current statement block?

(1) I do not think this approach will play terribly well in any of the
PLs; their notions of statement blocks all differ from whatever we might
think that is at the interactive-SQL-command level.

(2) AIUI the feature request is specifically for a single-statement
variable change, not any larger scope than that.

			regards, tom lane

  48. Re: PoC plpgsql - possibility to force custom or generic plan

    David G. Johnston <david.g.johnston@gmail.com> — 2017-09-08T21:16:05Z

    On Fri, Sep 8, 2017 at 2:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > personally I prefer syntax without FOR keyword - because following
> keyword
> > must be reserved keyword
>
> > SET x = .., y = .. SELECT ... ;
>
> Nope.  Most of the statement-starting keywords are *not* fully reserved;
> they don't need to be as long as they lead off the statement.  But this
> proposal would break that.  We need to put FOR or IN or another
> already-fully-reserved keyword after the SET list, or something's going
> to bite us.
>

Just throwing it ​out there but can we making putting SET inside a CTE work?

David J.

  49. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-09T05:34:48Z

    > >
> > SET x = .., y = .. SELECT ... ;
>
> This seems pretty ugly from a syntax perspective.
>
> We already have 'SET LOCAL', which manages scope to the current
> transaction.  How about SET BLOCK which would set until you've left
> the current statement block?
>

This is reason why PRAGMA was designed - it can works on function, block,
or statement level. But only in block based PL languages.


>
> merlin
>

  50. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-10T06:30:34Z

    2017-09-08 23:09 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > personally I prefer syntax without FOR keyword - because following
> keyword
> > must be reserved keyword
>
> > SET x = .., y = .. SELECT ... ;
>
> Nope.  Most of the statement-starting keywords are *not* fully reserved;
> they don't need to be as long as they lead off the statement.  But this
> proposal would break that.  We need to put FOR or IN or another
> already-fully-reserved keyword after the SET list, or something's going
> to bite us.
>

the possibility to control plan cache for any command via GUC outside
PLpgSQL can introduce lot of question.

What impact will be on PREPARE command and on EXECUTE command?

If we disable plan cache for EXECUTE, should we remove plan from plan
cache? ...

Can we have some diagnostic to get info if some command has cached plan or
not?

Regards

Pavel


>
>                         regards, tom lane
>

  51. Re: PoC plpgsql - possibility to force custom or generic plan

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-09-11T12:39:19Z

    On 9/8/17 13:14, Simon Riggs wrote:
> 2. Allow a SET to apply only for a single statement
> SET guc1 = x, guc2 = y FOR stmt
> e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
> Internally a GUC setting already exists for a single use, via
> GUC_ACTION_SAVE, so we just need to invoke it.

This doesn't read well to me.  It indicates to me "make this setting for
this query [in case I run it later]", but it does not indicate that the
query will be run.

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

  52. Re: PoC plpgsql - possibility to force custom or generic plan

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-11T12:44:44Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 9/8/17 13:14, Simon Riggs wrote:
>> 2. Allow a SET to apply only for a single statement
>> SET guc1 = x, guc2 = y FOR stmt
>> e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
>> Internally a GUC setting already exists for a single use, via
>> GUC_ACTION_SAVE, so we just need to invoke it.

> This doesn't read well to me.  It indicates to me "make this setting for
> this query [in case I run it later]", but it does not indicate that the
> query will be run.

Robert's original LET ... IN ... syntax proposal might be better from that
standpoint.

			regards, tom lane

  53. Re: PoC plpgsql - possibility to force custom or generic plan

    Bruce Momjian <bruce@momjian.us> — 2017-09-16T16:55:32Z

    On Wed, Sep  6, 2017 at 10:43:39AM -0400, Robert Haas wrote:
> helps.  I don't think we can just indefinitely continue to resist
> providing manual control over this behavior on the theory that some
> day we'll fix it.  It's been six years and we haven't made any
> significant progress.  In some cases, a long delay without any
> progress might just point to a lack of effort that should have been
> applied, but in this case I think it's because the problem is
> incredibly hard.

Add to that, we didn't even document the behavior until last year:

	commit fab9d1da4a213fab08fe2d263eedf2408bc4a27a
	Author: Bruce Momjian <bruce@momjian.us>
	Date:   Tue Jun 14 16:11:46 2016 -0400
	
	    document when PREPARE uses generic plans
	
	    Also explain how generic plans are created.
	    Link to PREPARE docs from wire-protocol prepare docs.
	
	    Reported-by: Jonathan Rogers
	
	    Discussion: https://www.postgresql.org/message-id/flat/561E749D.4090301%40socialserve.com

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

  54. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-19T03:46:26Z

    2017-09-11 14:44 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > On 9/8/17 13:14, Simon Riggs wrote:
> >> 2. Allow a SET to apply only for a single statement
> >> SET guc1 = x, guc2 = y FOR stmt
> >> e.g. SET max_parallel_workers = 4 FOR SELECT count(*) FROM bigtable
> >> Internally a GUC setting already exists for a single use, via
> >> GUC_ACTION_SAVE, so we just need to invoke it.
>
> > This doesn't read well to me.  It indicates to me "make this setting for
> > this query [in case I run it later]", but it does not indicate that the
> > query will be run.
>
> Robert's original LET ... IN ... syntax proposal might be better from that
> standpoint.
>

From my perspective Robert's proposal is not targeted to PLpgSQL well,
because it doesn't allow to choose granularity.

I am not sure what is result from this discussion:

1. this feature is wanted

2. a opened question is the syntax

I am sure so GUC are not a good design solution for PL/pgSQL. Robert's
proposal does thing  bit better, but it has sense more for another
environments than PLpgSQL - more, it allows more degree of freedom what has
sense for PLpgSQL.

There is possibility to introduce new compile option #option to disable
plan cache on function scope. Do you think so it is acceptable solution? It
is step forward.

Regards

Pavel



>                         regards, tom lane
>

  55. Re: PoC plpgsql - possibility to force custom or generic plan

    Robert Haas <robertmhaas@gmail.com> — 2017-09-19T16:33:52Z

    On Mon, Sep 18, 2017 at 11:46 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> There is possibility to introduce new compile option #option to disable plan
> cache on function scope. Do you think so it is acceptable solution? It is
> step forward.

You can already set a GUC with function scope.  I'm not getting your point.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  56. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-19T16:45:09Z

    2017-09-19 18:33 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

> On Mon, Sep 18, 2017 at 11:46 PM, Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
> > There is possibility to introduce new compile option #option to disable
> plan
> > cache on function scope. Do you think so it is acceptable solution? It is
> > step forward.
>
> You can already set a GUC with function scope.  I'm not getting your point.
>

yes, it is true. But implementation of #option is limited to PLpgSQL - so
there is not any too much questions - GUC is global - there is lot of
points:

* what is correct impact on PREPARE
* what is correct impact on EXECUTE
* what should be done if this GUC is changed ..

Regards

Pavel


> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

  57. Re: PoC plpgsql - possibility to force custom or generic plan

    Robert Haas <robertmhaas@gmail.com> — 2017-09-19T18:37:36Z

    On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> You can already set a GUC with function scope.  I'm not getting your
>> point.
>
> yes, it is true. But implementation of #option is limited to PLpgSQL - so
> there is not any too much questions - GUC is global - there is lot of
> points:
>
> * what is correct impact on PREPARE
> * what is correct impact on EXECUTE
> * what should be done if this GUC is changed ..

For better or for worse, as a project we've settled on GUCs as a way
to control behavior.  I think it makes more sense to try to apply that
option to new behaviors we want to control than to invent some new
system.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  58. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-09-19T18:45:06Z

    2017-09-19 20:37 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

> On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
> >> You can already set a GUC with function scope.  I'm not getting your
> >> point.
> >
> > yes, it is true. But implementation of #option is limited to PLpgSQL - so
> > there is not any too much questions - GUC is global - there is lot of
> > points:
> >
> > * what is correct impact on PREPARE
> > * what is correct impact on EXECUTE
> > * what should be done if this GUC is changed ..
>
> For better or for worse, as a project we've settled on GUCs as a way
> to control behavior.  I think it makes more sense to try to apply that
> option to new behaviors we want to control than to invent some new
> system.
>

I have nothing against GUC generally - just in this case, I have knowleadge
what is expected behave in plpgsql environment and I miss this knowleadge
else where, so I am thinking be good start just for plpgsql (where this
issue is mentioned often). The some plpgsql limitted implementation is not
barier against any another implementation.



> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

  59. Re: PoC plpgsql - possibility to force custom or generic plan

    Merlin Moncure <mmoncure@gmail.com> — 2017-09-19T18:49:56Z

    On Tue, Sep 19, 2017 at 1:37 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>> You can already set a GUC with function scope.  I'm not getting your
>>> point.
>>
>> yes, it is true. But implementation of #option is limited to PLpgSQL - so
>> there is not any too much questions - GUC is global - there is lot of
>> points:
>>
>> * what is correct impact on PREPARE
>> * what is correct impact on EXECUTE
>> * what should be done if this GUC is changed ..
>
> For better or for worse, as a project we've settled on GUCs as a way
> to control behavior.  I think it makes more sense to try to apply that
> option to new behaviors we want to control than to invent some new
> system.

This seems very sensible.

We also have infrastructure at the SQL level (SET) to manage the GUC.
Tom upthread (for pretty good reasons) extending SET to pl/pgsql
specific scoping but TBH I'm struggling as to why we need to implement
new syntax for this; the only thing missing is being able to scope SET
statements to a code block FWICT.

merlin

  60. Re: PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-10-12T09:31:29Z

    2017-09-19 20:49 GMT+02:00 Merlin Moncure <mmoncure@gmail.com>:

> On Tue, Sep 19, 2017 at 1:37 PM, Robert Haas <robertmhaas@gmail.com>
> wrote:
> > On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
> >>> You can already set a GUC with function scope.  I'm not getting your
> >>> point.
> >>
> >> yes, it is true. But implementation of #option is limited to PLpgSQL -
> so
> >> there is not any too much questions - GUC is global - there is lot of
> >> points:
> >>
> >> * what is correct impact on PREPARE
> >> * what is correct impact on EXECUTE
> >> * what should be done if this GUC is changed ..
> >
> > For better or for worse, as a project we've settled on GUCs as a way
> > to control behavior.  I think it makes more sense to try to apply that
> > option to new behaviors we want to control than to invent some new
> > system.
>
> This seems very sensible.
>
> We also have infrastructure at the SQL level (SET) to manage the GUC.
> Tom upthread (for pretty good reasons) extending SET to pl/pgsql
> specific scoping but TBH I'm struggling as to why we need to implement
> new syntax for this; the only thing missing is being able to scope SET
> statements to a code block FWICT.
>
>
here is a GUC based patch for plancache controlling. Looks so this code is
working.

It is hard to create regress tests. Any ideas?

Regards

Pavel



> merlin
>

  61. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Michael Paquier <michael.paquier@gmail.com> — 2017-11-30T03:12:26Z

    On Thu, Oct 12, 2017 at 6:31 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> It is hard to create regress tests. Any ideas?

Moving to next CF per lack of reviews.
-- 
Michael

  62. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Stephen Frost <sfrost@snowman.net> — 2018-01-22T22:15:43Z

    Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:
> here is a GUC based patch for plancache controlling. Looks so this code is
> working.

This really could use a new thread, imv.  This thread is a year old and
about a completely different feature than what you've implemented here.

> It is hard to create regress tests. Any ideas?

Honestly, my idea for this would be to add another option to EXPLAIN
which would make it spit out generic and custom plans, or something of
that sort.

Please review your patches before sending them and don't send in patches
which have random unrelated whitespace changes.

> diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
> index ad8a82f1e3..cc99cf6dcc 100644
> --- a/src/backend/utils/cache/plancache.c
> +++ b/src/backend/utils/cache/plancache.c
> @@ -1031,6 +1033,12 @@ choose_custom_plan(CachedPlanSource *plansource, ParamListInfo boundParams)
>  	if (IsTransactionStmtPlan(plansource))
>  		return false;
>  
> +	/* See if settings wants to force the decision */
> +	if (plancache_mode & PLANCACHE_FORCE_GENERIC_PLAN)
> +		return false;
> +	if (plancache_mode & PLANCACHE_FORCE_CUSTOM_PLAN)
> +		return true;

Not a big deal, but I probably would have just expanded the conditional
checks against cursor_options (just below) rather than adding
independent if() statements.

> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> index ae22185fbd..4ce275e39d 100644
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -3916,6 +3923,16 @@ static struct config_enum ConfigureNamesEnum[] =
>  		NULL, NULL, NULL
>  	},
>  
> +	{
> +		{"plancache_mode", PGC_USERSET, QUERY_TUNING_OTHER,
> +			gettext_noop("Forces use of custom or generic plans."),
> +			gettext_noop("It can control query plan cache.")
> +		},
> +		&plancache_mode,
> +		PLANCACHE_DEFAULT, plancache_mode_options,
> +		NULL, NULL, NULL
> +	},

That long description is shorter than the short description.  How about:

short: Controls the planner's selection of custom or generic plan.
long: Prepared queries have custom and generic plans and the planner
will attempt to choose which is better; this can be set to override
the default behavior.

(either that, or just ditch the long description)

> diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
> index 87fab19f3c..962895cc1a 100644
> --- a/src/include/utils/plancache.h
> +++ b/src/include/utils/plancache.h
> @@ -143,7 +143,6 @@ typedef struct CachedPlan
>  	MemoryContext context;		/* context containing this CachedPlan */
>  } CachedPlan;
>  
> -
>  extern void InitPlanCache(void);
>  extern void ResetPlanCache(void);
>  

Random unrelated whitespace change...

This is also missing documentation updates.

Setting to Waiting for Author, but with those changes I would think we
could mark it ready-for-committer, it seems pretty straight-forward to
me and there seems to be general agreement (now) that it's worthwhile to
have.

Thanks!

Stephen

  63. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Robert Haas <robertmhaas@gmail.com> — 2018-01-23T14:35:23Z

    On Mon, Jan 22, 2018 at 5:15 PM, Stephen Frost <sfrost@snowman.net> wrote:
> Pavel,
>
> * Pavel Stehule (pavel.stehule@gmail.com) wrote:
>> here is a GUC based patch for plancache controlling. Looks so this code is
>> working.
>
> This really could use a new thread, imv.  This thread is a year old and
> about a completely different feature than what you've implemented here.

+ if (plancache_mode & PLANCACHE_FORCE_GENERIC_PLAN)
+ return false;
+ if (plancache_mode & PLANCACHE_FORCE_CUSTOM_PLAN)
+ return true;

This should be ==, not &.

I could explain why & happens to work, but I won't.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  64. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-01-23T14:50:40Z

    2018-01-23 15:35 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:

> On Mon, Jan 22, 2018 at 5:15 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > Pavel,
> >
> > * Pavel Stehule (pavel.stehule@gmail.com) wrote:
> >> here is a GUC based patch for plancache controlling. Looks so this code
> is
> >> working.
> >
> > This really could use a new thread, imv.  This thread is a year old and
> > about a completely different feature than what you've implemented here.
>
> + if (plancache_mode & PLANCACHE_FORCE_GENERIC_PLAN)
> + return false;
> + if (plancache_mode & PLANCACHE_FORCE_CUSTOM_PLAN)
> + return true;
>
> This should be ==, not &.
>
> I could explain why & happens to work, but I won't.
>

you have true, thank you

Pavel


> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

  65. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-01-23T16:08:56Z

    2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net>:

> Pavel,
>
> * Pavel Stehule (pavel.stehule@gmail.com) wrote:
> > here is a GUC based patch for plancache controlling. Looks so this code
> is
> > working.
>
> This really could use a new thread, imv.  This thread is a year old and
> about a completely different feature than what you've implemented here.
>

true, but now it is too late


> > It is hard to create regress tests. Any ideas?
>
> Honestly, my idea for this would be to add another option to EXPLAIN
> which would make it spit out generic and custom plans, or something of
> that sort.
>
>
I looked there, but it needs cycle dependency between CachedPlan and
PlannedStmt. It needs more code than this patch and code will not be nicer.
I try to do some game with prepared statements



> Please review your patches before sending them and don't send in patches
> which have random unrelated whitespace changes.
>
> > diff --git a/src/backend/utils/cache/plancache.c
> b/src/backend/utils/cache/plancache.c
> > index ad8a82f1e3..cc99cf6dcc 100644
> > --- a/src/backend/utils/cache/plancache.c
> > +++ b/src/backend/utils/cache/plancache.c
> > @@ -1031,6 +1033,12 @@ choose_custom_plan(CachedPlanSource *plansource,
> ParamListInfo boundParams)
> >       if (IsTransactionStmtPlan(plansource))
> >               return false;
> >
> > +     /* See if settings wants to force the decision */
> > +     if (plancache_mode & PLANCACHE_FORCE_GENERIC_PLAN)
> > +             return false;
> > +     if (plancache_mode & PLANCACHE_FORCE_CUSTOM_PLAN)
> > +             return true;
>
> Not a big deal, but I probably would have just expanded the conditional
> checks against cursor_options (just below) rather than adding
> independent if() statements.
>

I don't think so proposed change is better - My opinion is not strong - and
commiter can change it simply

>
> > diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> > index ae22185fbd..4ce275e39d 100644
> > --- a/src/backend/utils/misc/guc.c
> > +++ b/src/backend/utils/misc/guc.c
> > @@ -3916,6 +3923,16 @@ static struct config_enum ConfigureNamesEnum[] =
> >               NULL, NULL, NULL
> >       },
> >
> > +     {
> > +             {"plancache_mode", PGC_USERSET, QUERY_TUNING_OTHER,
> > +                     gettext_noop("Forces use of custom or generic
> plans."),
> > +                     gettext_noop("It can control query plan cache.")
> > +             },
> > +             &plancache_mode,
> > +             PLANCACHE_DEFAULT, plancache_mode_options,
> > +             NULL, NULL, NULL
> > +     },
>
> That long description is shorter than the short description.  How about:
>
> short: Controls the planner's selection of custom or generic plan.
> long: Prepared queries have custom and generic plans and the planner
> will attempt to choose which is better; this can be set to override
> the default behavior.
>

changed - thank you for text

>
> (either that, or just ditch the long description)
>
> > diff --git a/src/include/utils/plancache.h
> b/src/include/utils/plancache.h
> > index 87fab19f3c..962895cc1a 100644
> > --- a/src/include/utils/plancache.h
> > +++ b/src/include/utils/plancache.h
> > @@ -143,7 +143,6 @@ typedef struct CachedPlan
> >       MemoryContext context;          /* context containing this
> CachedPlan */
> >  } CachedPlan;
> >
> > -
> >  extern void InitPlanCache(void);
> >  extern void ResetPlanCache(void);
> >
>
> Random unrelated whitespace change...
>

fixed

>
> This is also missing documentation updates.
>

I wrote some basic doc, but native speaker should to write more words about
used strategies.


> Setting to Waiting for Author, but with those changes I would think we
> could mark it ready-for-committer, it seems pretty straight-forward to
> me and there seems to be general agreement (now) that it's worthwhile to
> have.
>
> Thanks!
>

attached updated patch

Regards

Pavel


> Stephen
>

  66. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp> — 2018-01-30T08:06:16Z

    On 2018/01/24 1:08, Pavel Stehule wrote:
> 
> 
> 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net <mailto:sfrost@snowman.net>>:
> 
>     Pavel,
> 
>     * Pavel Stehule (pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>) wrote:
>     > here is a GUC based patch for plancache controlling. Looks so this code is
>     > working.
> 
>     This really could use a new thread, imv.  This thread is a year old and
>     about a completely different feature than what you've implemented here.
> 
> 
> true, but now it is too late
> 
> 
>     > It is hard to create regress tests. Any ideas?
> 
>     Honestly, my idea for this would be to add another option to EXPLAIN
>     which would make it spit out generic and custom plans, or something of
>     that sort.
> 
> 
> I looked there, but it needs cycle dependency between CachedPlan and PlannedStmt. It needs more code than this patch and code will not be nicer. I try to do some game with prepared statements
> 
>     Please review your patches before sending them and don't send in patches
>     which have random unrelated whitespace changes.
> 
>      > diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
>      > index ad8a82f1e3..cc99cf6dcc 100644
>      > --- a/src/backend/utils/cache/plancache.c
>      > +++ b/src/backend/utils/cache/plancache.c
>      > @@ -1031,6 +1033,12 @@ choose_custom_plan(CachedPlanSource *plansource, ParamListInfo boundParams)
>      >       if (IsTransactionStmtPlan(plansource))
>      >               return false;
>      >
>      > +     /* See if settings wants to force the decision */
>      > +     if (plancache_mode & PLANCACHE_FORCE_GENERIC_PLAN)
>      > +             return false;
>      > +     if (plancache_mode & PLANCACHE_FORCE_CUSTOM_PLAN)
>      > +             return true;
> 
>     Not a big deal, but I probably would have just expanded the conditional
>     checks against cursor_options (just below) rather than adding
>     independent if() statements.
> 
> 
> I don't think so proposed change is better - My opinion is not strong - and commiter can change it simply
> 
> 
>      > diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
>      > index ae22185fbd..4ce275e39d 100644
>      > --- a/src/backend/utils/misc/guc.c
>      > +++ b/src/backend/utils/misc/guc.c
>      > @@ -3916,6 +3923,16 @@ static struct config_enum ConfigureNamesEnum[] =
>      >               NULL, NULL, NULL
>      >       },
>      >
>      > +     {
>      > +             {"plancache_mode", PGC_USERSET, QUERY_TUNING_OTHER,
>      > +                     gettext_noop("Forces use of custom or generic plans."),
>      > +                     gettext_noop("It can control query plan cache.")
>      > +             },
>      > +             &plancache_mode,
>      > +             PLANCACHE_DEFAULT, plancache_mode_options,
>      > +             NULL, NULL, NULL
>      > +     },
> 
>     That long description is shorter than the short description.  How about:
> 
>     short: Controls the planner's selection of custom or generic plan.
>     long: Prepared queries have custom and generic plans and the planner
>     will attempt to choose which is better; this can be set to override
>     the default behavior.
> 
> 
> changed - thank you for text
> 
> 
>     (either that, or just ditch the long description)
> 
>      > diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
>      > index 87fab19f3c..962895cc1a 100644
>      > --- a/src/include/utils/plancache.h
>      > +++ b/src/include/utils/plancache.h
>      > @@ -143,7 +143,6 @@ typedef struct CachedPlan
>      >       MemoryContext context;          /* context containing this CachedPlan */
>      >  } CachedPlan;
>      >
>      > -
>      >  extern void InitPlanCache(void);
>      >  extern void ResetPlanCache(void);
>      >
> 
>     Random unrelated whitespace change...
> 
> 
> fixed
> 
> 
>     This is also missing documentation updates.
> 
> 
> I wrote some basic doc, but native speaker should to write more words about used strategies.
> 
> 
>     Setting to Waiting for Author, but with those changes I would think we
>     could mark it ready-for-committer, it seems pretty straight-forward to
>     me and there seems to be general agreement (now) that it's worthwhile to
>     have.
> 
>     Thanks!
> 
> 
> attached updated patch
> 
> Regards
> 
> Pavel
> 
> 
>     Stephen

Hi Pavel,

I tested your patch on a044378ce2f6268a996c8cce2b7bfb5d82b05c90.
I share my results to you.

  - Result of patch command

     $ patch -p1 < guc-plancache_mode-180123.patch
     patching file doc/src/sgml/config.sgml
     Hunk #1 succeeded at 4400 (offset 13 lines).
     patching file src/backend/utils/cache/plancache.c
     patching file src/backend/utils/misc/guc.c
     patching file src/include/utils/plancache.h
     patching file src/test/regress/expected/plancache.out
     patching file src/test/regress/sql/plancache.sql

  - Result of regression test

     =======================
      All 186 tests passed.
     =======================

Regards,
Tatsuro Yamada

  67. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-01-30T13:59:35Z

    2018-01-30 9:06 GMT+01:00 Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>:

> On 2018/01/24 1:08, Pavel Stehule wrote:
>
>>
>>
>> 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net <mailto:
>> sfrost@snowman.net>>:
>>
>>     Pavel,
>>
>>
>>     * Pavel Stehule (pavel.stehule@gmail.com <mailto:
>> pavel.stehule@gmail.com>) wrote:
>>     > here is a GUC based patch for plancache controlling. Looks so this
>> code is
>>     > working.
>>
>>     This really could use a new thread, imv.  This thread is a year old
>> and
>>     about a completely different feature than what you've implemented
>> here.
>>
>>
>> true, but now it is too late
>>
>>
>>     > It is hard to create regress tests. Any ideas?
>>
>>     Honestly, my idea for this would be to add another option to EXPLAIN
>>     which would make it spit out generic and custom plans, or something of
>>     that sort.
>>
>>
>> I looked there, but it needs cycle dependency between CachedPlan and
>> PlannedStmt. It needs more code than this patch and code will not be nicer.
>> I try to do some game with prepared statements
>>
>>     Please review your patches before sending them and don't send in
>> patches
>>     which have random unrelated whitespace changes.
>>
>>      > diff --git a/src/backend/utils/cache/plancache.c
>> b/src/backend/utils/cache/plancache.c
>>      > index ad8a82f1e3..cc99cf6dcc 100644
>>      > --- a/src/backend/utils/cache/plancache.c
>>      > +++ b/src/backend/utils/cache/plancache.c
>>      > @@ -1031,6 +1033,12 @@ choose_custom_plan(CachedPlanSource
>> *plansource, ParamListInfo boundParams)
>>      >       if (IsTransactionStmtPlan(plansource))
>>      >               return false;
>>      >
>>      > +     /* See if settings wants to force the decision */
>>      > +     if (plancache_mode & PLANCACHE_FORCE_GENERIC_PLAN)
>>      > +             return false;
>>      > +     if (plancache_mode & PLANCACHE_FORCE_CUSTOM_PLAN)
>>      > +             return true;
>>
>>     Not a big deal, but I probably would have just expanded the
>> conditional
>>     checks against cursor_options (just below) rather than adding
>>     independent if() statements.
>>
>>
>> I don't think so proposed change is better - My opinion is not strong -
>> and commiter can change it simply
>>
>>
>>      > diff --git a/src/backend/utils/misc/guc.c
>> b/src/backend/utils/misc/guc.c
>>      > index ae22185fbd..4ce275e39d 100644
>>      > --- a/src/backend/utils/misc/guc.c
>>      > +++ b/src/backend/utils/misc/guc.c
>>      > @@ -3916,6 +3923,16 @@ static struct config_enum
>> ConfigureNamesEnum[] =
>>      >               NULL, NULL, NULL
>>      >       },
>>      >
>>      > +     {
>>      > +             {"plancache_mode", PGC_USERSET, QUERY_TUNING_OTHER,
>>      > +                     gettext_noop("Forces use of custom or
>> generic plans."),
>>      > +                     gettext_noop("It can control query plan
>> cache.")
>>      > +             },
>>      > +             &plancache_mode,
>>      > +             PLANCACHE_DEFAULT, plancache_mode_options,
>>      > +             NULL, NULL, NULL
>>      > +     },
>>
>>     That long description is shorter than the short description.  How
>> about:
>>
>>     short: Controls the planner's selection of custom or generic plan.
>>     long: Prepared queries have custom and generic plans and the planner
>>     will attempt to choose which is better; this can be set to override
>>     the default behavior.
>>
>>
>> changed - thank you for text
>>
>>
>>     (either that, or just ditch the long description)
>>
>>      > diff --git a/src/include/utils/plancache.h
>> b/src/include/utils/plancache.h
>>      > index 87fab19f3c..962895cc1a 100644
>>      > --- a/src/include/utils/plancache.h
>>      > +++ b/src/include/utils/plancache.h
>>      > @@ -143,7 +143,6 @@ typedef struct CachedPlan
>>      >       MemoryContext context;          /* context containing this
>> CachedPlan */
>>      >  } CachedPlan;
>>      >
>>      > -
>>      >  extern void InitPlanCache(void);
>>      >  extern void ResetPlanCache(void);
>>      >
>>
>>     Random unrelated whitespace change...
>>
>>
>> fixed
>>
>>
>>     This is also missing documentation updates.
>>
>>
>> I wrote some basic doc, but native speaker should to write more words
>> about used strategies.
>>
>>
>>     Setting to Waiting for Author, but with those changes I would think we
>>     could mark it ready-for-committer, it seems pretty straight-forward to
>>     me and there seems to be general agreement (now) that it's worthwhile
>> to
>>     have.
>>
>>     Thanks!
>>
>>
>> attached updated patch
>>
>> Regards
>>
>> Pavel
>>
>>
>>     Stephen
>>
>
> Hi Pavel,
>
> I tested your patch on a044378ce2f6268a996c8cce2b7bfb5d82b05c90.
> I share my results to you.
>
>  - Result of patch command
>
>     $ patch -p1 < guc-plancache_mode-180123.patch
>     patching file doc/src/sgml/config.sgml
>     Hunk #1 succeeded at 4400 (offset 13 lines).
>     patching file src/backend/utils/cache/plancache.c
>     patching file src/backend/utils/misc/guc.c
>     patching file src/include/utils/plancache.h
>     patching file src/test/regress/expected/plancache.out
>     patching file src/test/regress/sql/plancache.sql
>
>  - Result of regression test
>
>     =======================
>      All 186 tests passed.
>     =======================
>
>
Thank you very much

Regards

Pavel


> Regards,
> Tatsuro Yamada
>
>

  68. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Andres Freund <andres@anarazel.de> — 2018-03-01T22:10:53Z

    On 2018-01-23 17:08:56 +0100, Pavel Stehule wrote:
> 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net>:
> > This really could use a new thread, imv.  This thread is a year old and
> > about a completely different feature than what you've implemented here.
> >
> 
> true, but now it is too late

At the very least the CF entry could be renamed moved out the procedual
language category?

  69. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-03-02T02:13:04Z

    2018-03-01 23:10 GMT+01:00 Andres Freund <andres@anarazel.de>:

> On 2018-01-23 17:08:56 +0100, Pavel Stehule wrote:
> > 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net>:
> > > This really could use a new thread, imv.  This thread is a year old and
> > > about a completely different feature than what you've implemented here.
> > >
> >
> > true, but now it is too late
>
> At the very least the CF entry could be renamed moved out the procedual
> language category?
>

Why not? Have you idea, what category is best?

Regards

Pavel

  70. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Andres Freund <andres@anarazel.de> — 2018-03-02T02:38:52Z

    On 2018-03-02 03:13:04 +0100, Pavel Stehule wrote:
> 2018-03-01 23:10 GMT+01:00 Andres Freund <andres@anarazel.de>:
> 
> > On 2018-01-23 17:08:56 +0100, Pavel Stehule wrote:
> > > 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net>:
> > > > This really could use a new thread, imv.  This thread is a year old and
> > > > about a completely different feature than what you've implemented here.
> > > >
> > >
> > > true, but now it is too late
> >
> > At the very least the CF entry could be renamed moved out the procedual
> > language category?
> >
> 
> Why not?

Because the patch adds GUCs that don't have a direct connection
toprocedual languages?  And the patch's topic still says "plpgsql plan
cache behave" which surely is misleading.

Seems fairly obvious that neither category nor name is particularly
descriptive of the current state?


> Have you idea, what category is best?

Server Features? Misc?  And as a title something about "add GUCs to
control custom plan logic"?


Greetings,

Andres Freund

  71. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-03-02T02:43:56Z

    2018-03-02 3:38 GMT+01:00 Andres Freund <andres@anarazel.de>:

> On 2018-03-02 03:13:04 +0100, Pavel Stehule wrote:
> > 2018-03-01 23:10 GMT+01:00 Andres Freund <andres@anarazel.de>:
> >
> > > On 2018-01-23 17:08:56 +0100, Pavel Stehule wrote:
> > > > 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net>:
> > > > > This really could use a new thread, imv.  This thread is a year
> old and
> > > > > about a completely different feature than what you've implemented
> here.
> > > > >
> > > >
> > > > true, but now it is too late
> > >
> > > At the very least the CF entry could be renamed moved out the procedual
> > > language category?
> > >
> >
> > Why not?
>
> Because the patch adds GUCs that don't have a direct connection
> toprocedual languages?  And the patch's topic still says "plpgsql plan
> cache behave" which surely is misleading.
>
> Seems fairly obvious that neither category nor name is particularly
> descriptive of the current state?
>
>
ok


> > Have you idea, what category is best?
>
> Server Features? Misc?  And as a title something about "add GUCs to
> control custom plan logic"?
>
>
I'll move it there.

Regards

Pavel

>
> Greetings,
>
> Andres Freund
>

  72. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-03-02T21:34:42Z

    2018-03-02 3:43 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

>
>
> 2018-03-02 3:38 GMT+01:00 Andres Freund <andres@anarazel.de>:
>
>> On 2018-03-02 03:13:04 +0100, Pavel Stehule wrote:
>> > 2018-03-01 23:10 GMT+01:00 Andres Freund <andres@anarazel.de>:
>> >
>> > > On 2018-01-23 17:08:56 +0100, Pavel Stehule wrote:
>> > > > 2018-01-22 23:15 GMT+01:00 Stephen Frost <sfrost@snowman.net>:
>> > > > > This really could use a new thread, imv.  This thread is a year
>> old and
>> > > > > about a completely different feature than what you've implemented
>> here.
>> > > > >
>> > > >
>> > > > true, but now it is too late
>> > >
>> > > At the very least the CF entry could be renamed moved out the
>> procedual
>> > > language category?
>> > >
>> >
>> > Why not?
>>
>> Because the patch adds GUCs that don't have a direct connection
>> toprocedual languages?  And the patch's topic still says "plpgsql plan
>> cache behave" which surely is misleading.
>>
>> Seems fairly obvious that neither category nor name is particularly
>> descriptive of the current state?
>>
>>
> ok
>
>
>> > Have you idea, what category is best?
>>
>> Server Features? Misc?  And as a title something about "add GUCs to
>> control custom plan logic"?
>>
>>
> I'll move it there.
>

done

Pavel


>
> Regards
>
> Pavel
>
>>
>> Greetings,
>>
>> Andres Freund
>>
>
>

  73. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-07-10T10:01:00Z

    On 23.01.18 17:08, Pavel Stehule wrote:
> attached updated patch

This appears to be the patch of record in this thread.  I think there is
general desire for adding a setting like this, and the implementation is
simple enough.

One change perhaps: How about naming the default setting "auto" instead
of "default".  That makes it clearer what it does.

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

  74. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-07-12T09:12:59Z

    Hi

2018-07-10 12:01 GMT+02:00 Peter Eisentraut <
peter.eisentraut@2ndquadrant.com>:

> On 23.01.18 17:08, Pavel Stehule wrote:
> > attached updated patch
>
> This appears to be the patch of record in this thread.  I think there is
> general desire for adding a setting like this, and the implementation is
> simple enough.
>
> One change perhaps: How about naming the default setting "auto" instead
> of "default".  That makes it clearer what it does.
>

I changed "default" to "auto"

updated patch attached

Regards

Pavel


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

  75. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp> — 2018-07-12T10:45:47Z

    On 2018/07/12 18:12, Pavel Stehule wrote:
> Hi
> 
> 2018-07-10 12:01 GMT+02:00 Peter Eisentraut <peter.eisentraut@2ndquadrant.com <mailto:peter.eisentraut@2ndquadrant.com>>:
> 
>     On 23.01.18 17:08, Pavel Stehule wrote:
>      > attached updated patch
> 
>     This appears to be the patch of record in this thread.  I think there is
>     general desire for adding a setting like this, and the implementation is
>     simple enough.
> 
>     One change perhaps: How about naming the default setting "auto" instead
>     of "default".  That makes it clearer what it does.
> 
> 
> I changed "default" to "auto"
> 
> updated patch attached
> 
> Regards
> 
> Pavel

Hi Pavel,

I tested your patch on 40ca70eb.
Here is the result.

=======================
  All 190 tests passed.
=======================

Thanks,
Tatsuro Yamada

  76. Re: [HACKERS] PoC plpgsql - possibility to force custom or generic plan

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-07-16T12:07:43Z

    On 12.07.18 11:12, Pavel Stehule wrote:
>     This appears to be the patch of record in this thread.  I think there is
>     general desire for adding a setting like this, and the implementation is
>     simple enough.
> 
>     One change perhaps: How about naming the default setting "auto" instead
>     of "default".  That makes it clearer what it does.
> 
> 
> I changed "default" to "auto"
> 
> updated patch attached

committed with some editing

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