PoC plpgsql - possibility to force custom or generic plan

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
60 messages Options
123
Reply | Threaded
Open this post in threaded view
|

PoC plpgsql - possibility to force custom or generic plan

Pavel Stehule
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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

plpgsql-pragma-plan_cache.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Jim Nasby-5
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)


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule
Hi

2017-01-23 21:59 GMT+01:00 Jim Nasby <[hidden email]>:
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 <a href="tel:%28855-873-2532" value="+18558732532" target="_blank">(855-873-2532)

Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule
In reply to this post by Jim Nasby-5


2017-01-23 21:59 GMT+01:00 Jim Nasby <[hidden email]>:
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

Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule
In reply to this post by Pavel Stehule


2017-01-24 6:38 GMT+01:00 Pavel Stehule <[hidden email]>:
Hi

2017-01-23 21:59 GMT+01:00 Jim Nasby <[hidden email]>:
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 <a href="tel:%28855-873-2532" value="+18558732532" target="_blank">(855-873-2532)




--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

plpgsql-pragma-plan_cache-01.patch (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Jim Nasby-5
In reply to this post by Pavel Stehule
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)


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule


2017-01-25 21:06 GMT+01:00 Jim Nasby <[hidden email]>:
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 <a href="tel:%28855-873-2532" value="+18558732532" target="_blank">(855-873-2532)

Reply | Threaded
Open this post in threaded view
|

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

Greg Stark
In reply to this post by Jim Nasby-5
On 25 January 2017 at 20:06, Jim Nasby <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule
In reply to this post by Pavel Stehule
Hi

2017-01-24 21:33 GMT+01:00 Pavel Stehule <[hidden email]>:




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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

plpgsql-pragma-plan_cache-02.patch (24K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Jim Nasby-5
In reply to this post by Greg Stark
On 1/27/17 4:14 AM, Greg Stark wrote:
> On 25 January 2017 at 20:06, Jim Nasby <[hidden email]> 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)


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

David Steele
In reply to this post by Pavel Stehule
On 2/1/17 3:59 PM, Pavel Stehule wrote:

> Hi
>
> 2017-01-24 21:33 GMT+01:00 Pavel Stehule <[hidden email]
> <mailto:[hidden email]>>:
>
>             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
[hidden email]


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Petr Jelinek-4
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 <[hidden email]
>> <mailto:[hidden email]>>:
>>
>>             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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule


2017-03-18 19:30 GMT+01:00 Petr Jelinek <[hidden email]>:
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 <[hidden email]
>> <mailto:[hidden email]>>:
>>
>>             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

Reply | Threaded
Open this post in threaded view
|

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

Petr Jelinek-4
On 19/03/17 12:32, Pavel Stehule wrote:

>
>
> 2017-03-18 19:30 GMT+01:00 Petr Jelinek <[hidden email]
> <mailto:[hidden email]>>:
>
>     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 <[hidden email] <mailto:[hidden email]>
>     >> <mailto:[hidden email] <mailto:[hidden email]>>>:
>     >>
>     >>             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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule


2017-03-19 14:30 GMT+01:00 Petr Jelinek <[hidden email]>:
On 19/03/17 12:32, Pavel Stehule wrote:
>
>
> 2017-03-18 19:30 GMT+01:00 Petr Jelinek <[hidden email]
> <mailto:[hidden email]>>:
>
>     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 <[hidden email] <mailto:[hidden email]>
>     >> <mailto:[hidden email] <mailto:[hidden email]>>>:
>     >>
>     >>             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



--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

psql-pragma-plan_cache-01.patch (26K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule
Hi

rebased due last changes in pg_exec.c

Regards

Pavel


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

psql-pragma-plan_cache-02.patch (26K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Petr Jelinek-4
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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Add-plan_cache-control-PRAGMA-to-pl-pgsql.patch (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Pavel Stehule


2017-03-28 20:29 GMT+02:00 Petr Jelinek <[hidden email]>:
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

Reply | Threaded
Open this post in threaded view
|

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

Andres Freund
In reply to this post by Petr Jelinek-4
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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

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

Tom Lane-2
Andres Freund <[hidden email]> 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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
123
Previous Thread Next Thread