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

[Pavel Stehule]

2017-09-19 20:49 GMT+02:00 Merlin Moncure :

> On Tue, Sep 19, 2017 at 1:37 PM, Robert Haas 
> wrote:
> > On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule 
> 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
>
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
@@ -106,6 +106,8 @@ static void PlanCacheRelCallback(Datum arg, Oid relid);
 static void PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue);
 static void PlanCacheSysCallback(Datum arg, int cacheid, uint32 hashvalue);
 
+/* GUC parameter */
+int	plancache_mode;
 
 /*
  * InitPlanCache: initialize module during InitPostgres.
@@ -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;
+
 	/* See if caller wants to force the decision */
 	if (plansource->cursor_options & CURSOR_OPT_GENERIC_PLAN)
 		return false;
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
@@ -403,6 +403,13 @@ static const struct config_enum_entry force_parallel_mode_options[] = {
 	{NULL, 0, false}
 };
 
+static const struct config_enum_entry plancache_mode_options[] = {
+	{"default", PLANCACHE_DEFAULT, false},
+	{"force_generic_plan", PLANCACHE_FORCE_GENERIC_PLAN, false},
+	{"force_custom_plan", PLANCACHE_FORCE_CUSTOM_PLAN, false},
+	{NULL, 0, false}
+};
+
 /*
  * password_encryption used to be a boolean, so accept all the likely
  * variants of "on", too. "off" used to store passwords in plaintext,
@@ -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.")
+		},
+		_mode,
+		PLANCACHE_DEFAULT, plancache_mode_options,
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, 0, NULL, NULL, NULL, NULL
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);
 
@@ -182,4 +181,16 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
 			  QueryEnvironment *queryEnv);
 extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner);
 
+/* possible values for plancache_mode */
+typedef enum
+{
+	PLANCACHE_DEFAULT,
+	PLANCACHE_FORCE_GENERIC_PLAN,
+	PLANCACHE_FORCE_CUSTOM_PLAN
+}			PlanCacheMode;
+
+
+/* GUC parameter */
+extern int plancache_mode;
+
 #endif			/* PLANCACHE_H */

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

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

[Merlin Moncure]

On Tue, Sep 19, 2017 at 1:37 PM, Robert Haas  wrote:
> On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule  
> 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


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

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

[Pavel Stehule]

2017-09-19 20:37 GMT+02:00 Robert Haas :

> On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule 
> 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
>

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

[Robert Haas]

On Tue, Sep 19, 2017 at 12:45 PM, Pavel Stehule  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


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

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

[Pavel Stehule]

2017-09-19 18:33 GMT+02:00 Robert Haas :

> On Mon, Sep 18, 2017 at 11:46 PM, Pavel Stehule 
> 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
>

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

[Robert Haas]

On Mon, Sep 18, 2017 at 11:46 PM, Pavel Stehule  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


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

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

[Pavel Stehule]

2017-09-11 14:44 GMT+02:00 Tom Lane :

> Peter Eisentraut  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
>

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

[Bruce Momjian]

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


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

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

[Tom Lane]

Peter Eisentraut  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


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

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

[Peter Eisentraut]

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


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

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

[Pavel Stehule]

2017-09-08 23:09 GMT+02:00 Tom Lane :

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

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

[Pavel Stehule]

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

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

[David G. Johnston]

On Fri, Sep 8, 2017 at 2:09 PM, Tom Lane  wrote:

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

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

[Tom Lane]

Merlin Moncure  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


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

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

[Tom Lane]

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


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

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

[Merlin Moncure]

On Fri, Sep 8, 2017 at 2:48 PM, Pavel Stehule  wrote:
>
>
> 2017-09-08 21:21 GMT+02:00 Daniel Gustafsson :
>>
>> > On 08 Sep 2017, at 19:14, Simon Riggs  wrote:
>> >
>> > On 6 September 2017 at 07:43, Robert Haas  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


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

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

[Pavel Stehule]

2017-09-08 21:21 GMT+02:00 Daniel Gustafsson :

> > On 08 Sep 2017, at 19:14, Simon Riggs  wrote:
> >
> > On 6 September 2017 at 07:43, Robert Haas  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
>

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

[Daniel Gustafsson]

> On 08 Sep 2017, at 19:14, Simon Riggs  wrote:
> 
> On 6 September 2017 at 07:43, Robert Haas  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


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

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

[Pavel Stehule]

2017-09-08 19:14 GMT+02:00 Simon Riggs :

> On 6 September 2017 at 07:43, Robert Haas  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 Riggshttp://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

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

[Simon Riggs]

On 6 September 2017 at 07:43, Robert Haas  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 Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

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

[Simon Riggs]

On 6 September 2017 at 07:43, Robert Haas  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 Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

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

[Pavel Stehule]

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

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

[Pavel Stehule]

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

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

[Robert Haas]

On Wed, Sep 6, 2017 at 11:03 AM, Tom Lane  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


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

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

[Tom Lane]

Robert Haas  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


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

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

[Robert Haas]

On Tue, Sep 5, 2017 at 1:38 PM, Tom Lane  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


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

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

[Pavel Stehule]

2017-09-05 19:38 GMT+02:00 Tom Lane :

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

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

[Tom Lane]

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


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

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

[Pavel Stehule]

2017-09-05 15:01 GMT+02:00 Daniel Gustafsson :

> > On 08 Apr 2017, at 09:42, Pavel Stehule  wrote:
> >
> > 2017-04-08 2:30 GMT+02:00 Peter Eisentraut <
> peter.eisentr...@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-c8ed6a019...@2ndquadrant.com  message-id/659a2fce-b6ee-06de-05c0-c8ed6a019...@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

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

[Daniel Gustafsson]

> On 08 Apr 2017, at 09:42, Pavel Stehule  wrote:
> 
> 2017-04-08 2:30 GMT+02:00 Peter Eisentraut  >:
> 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-c8ed6a019...@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

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

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

[Pavel Stehule]

2017-04-08 2:30 GMT+02:00 Peter Eisentraut :

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

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

[Peter Eisentraut]

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


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

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

[Pavel Stehule]

2017-04-06 12:30 GMT+02:00 Andrew Dunstan :

>
>
> On 04/05/2017 05:41 PM, Andres Freund wrote:
> > On 2017-04-05 17:22:34 -0400, Tom Lane wrote:
> >> Andres Freund  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 Dunstanhttps://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

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

[Andrew Dunstan]

On 04/05/2017 05:41 PM, Andres Freund wrote:
> On 2017-04-05 17:22:34 -0400, Tom Lane wrote:
>> Andres Freund  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 Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



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

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

[Petr Jelinek]

On 05/04/17 23:22, Tom Lane wrote:
> Andres Freund  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


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

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

[Pavel Stehule]

2017-04-06 8:08 GMT+02:00 Pavel Stehule :

>
>
> 2017-04-05 23:22 GMT+02:00 Tom Lane :
>
>> Andres Freund  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
>>
>
>

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

[Pavel Stehule]

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

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

[Pavel Stehule]

2017-04-05 23:22 GMT+02:00 Tom Lane :

> Andres Freund  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
>

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

[Pavel Stehule]

2017-04-05 22:33 GMT+02:00 Andres Freund :

> Hi,
>
>
> I'd like some input from other committers whether we want this.  I'm
> somewhat doubtful, but don't have particularly strong feelings.
>
>
> > +
> > +  
> > +   Block level PRAGMA
> > +
> > +   
> > +PRAGMA
> > +in PL/pgSQL
> > +   
> > +
> > +   
> > +The block level PRAGMA allows to change the
> > +PL/pgSQL compiler behavior. Currently
> > +only PRAGMA PLAN_CACHE 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.


>
>
> > +
> > +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
>
>
> > +
> > + PRAGMA PLAN_CACHE
> > +
> > + 
> > +  The plan cache behavior can be controlled using
> > +  PRAGMA PLAN_CACHE. This PRAGMA can be
> used both
> > +  for whole function or in individual blocks. The following options
> are
>
> *functions
>
>
> > +  possible: DEFAULT - default
> > +  PL/pgSQL implementation - the system
> tries
> > +  to decide between custom plan and generic plan after five query
> > +  executions, FORCE_CUSTOM_PLAN - the chosen
> execution
> > +  plan will be the one shot plan - it is specific for every set of
> > +  used paramaters, FORCE_GENERIC_PLAN - 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.
>
>
> > + 
> > +
> > + 
> > +  
> > +   PRAGMA PLAN_CACHE
> > +   in PL/pgSQL
> > +  
> > +  The plan for INSERT 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 _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
>

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

[Andres Freund]

On 2017-04-05 17:22:34 -0400, Tom Lane wrote:
> Andres Freund  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


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

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

[Tom Lane]

Andres Freund  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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Andres Freund]

Hi,


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


> +
> +  
> +   Block level PRAGMA
> +
> +   
> +PRAGMA
> +in PL/pgSQL
> +   
> +
> +   
> +The block level PRAGMA allows to change the
> +PL/pgSQL compiler behavior. Currently
> +only PRAGMA PLAN_CACHE is supported.

Why are we doing this on a block level?


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


> +
> + PRAGMA PLAN_CACHE
> +
> + 
> +  The plan cache behavior can be controlled using
> +  PRAGMA PLAN_CACHE. This PRAGMA can be used both
> +  for whole function or in individual blocks. The following options are

*functions


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


> + 
> +
> + 
> +  
> +   PRAGMA PLAN_CACHE
> +   in PL/pgSQL
> +  
> +  The plan for INSERT 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 _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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Pavel Stehule]

2017-03-28 20:29 GMT+02:00 Petr Jelinek :

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

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

[Petr Jelinek]

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


Add-plan_cache-control-PRAGMA-to-pl-pgsql.patch
Description: binary/octet-stream

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

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

[Pavel Stehule]

Hi

rebased due last changes in pg_exec.c

Regards

Pavel
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d356deb9f5..56da4d6163 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -802,6 +802,32 @@ $$ LANGUAGE plpgsql;
 happen in a plain SQL command.

   
+
+  
+   Block level PRAGMA
+
+   
+PRAGMA
+in PL/pgSQL
+   
+
+   
+The block level PRAGMA allows to change some
+PL/pgSQL compiler behave. Currently
+only PRAGMA PLAN_CACHE is supported.
+
+
+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
+  RETURN EXISTS(SELECT * FROM tab WHERE id = _id);
+END;
+$$ LANGUAGE plpgsql;
+
+   
+  
   
 
   
@@ -4649,6 +4675,43 @@ $$ LANGUAGE plpgsql;
  use of the now() function would still be a better idea.
 
 
+
+
+ PRAGMA PLAN_CACHE
+
+ 
+  The plan cache behave can be controlled with PRAGMA PLAN_CACHE.
+  This PRAGMA can be used on function or on block level (per
+  function, per block). The following options are possible:
+  DEFAULT - default PL/pgSQL
+  implementation - the system try to decide between custom plan and generic
+  plan after five query executions, FORCE_CUSTOM_PLAN
+  - the execution plan is one shot plan - it is specific for every set of
+  used paramaters, FORCE_GENERIC_PLAN - the query plan
+  is generic from start.
+ 
+
+ 
+  
+   PRAGMA PLAN_CACHE
+   in PL/pgSQL
+  
+  The plan for INSERT is generic from begin. The 
+  PRAGMA PLAN_CACHE is related to function - etc. every command
+  in this function will use generic plan.
+
+CREATE FUNCTION logfunc2(logtxt text) RETURNS void AS $$
+PRAGMA PLAN_CACHE(FORCE_GENERIC_PLAN);
+DECLARE
+curtime timestamp;
+BEGIN
+curtime := 'now';
+INSERT INTO logtable VALUES (logtxt, curtime);
+END;
+$$ LANGUAGE plpgsql;
+
+ 
+
   
 
   
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index bed343ea0c..70c970d20c 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -85,6 +85,10 @@ static const ExceptionLabelMap exception_label_map[] = {
 	{NULL, 0}
 };
 
+PLpgSQL_settings default_settings = {
+	NULL,
+	0			/* no special cursor option */
+};
 
 /* --
  * static prototypes
@@ -371,6 +375,7 @@ do_compile(FunctionCallInfo fcinfo,
 	 * outermost namespace contains function parameters and other special
 	 * variables (such as FOUND), and is named after the function itself.
 	 */
+	plpgsql_settings_init(_settings);
 	plpgsql_ns_init();
 	plpgsql_ns_push(NameStr(procStruct->proname), PLPGSQL_LABEL_BLOCK);
 	plpgsql_DumpExecTree = false;
@@ -849,6 +854,7 @@ plpgsql_compile_inline(char *proc_source)
 	function->extra_warnings = 0;
 	function->extra_errors = 0;
 
+	plpgsql_settings_init(_settings);
 	plpgsql_ns_init();
 	plpgsql_ns_push(func_name, PLPGSQL_LABEL_BLOCK);
 	plpgsql_DumpExecTree = false;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index c27935b51b..762ee6f70f 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2336,7 +2336,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
 	Assert(query);
 
 	if (query->plan == NULL)
-		exec_prepare_plan(estate, query, curvar->cursor_options);
+		exec_prepare_plan(estate, query, query->cursor_options);
 
 	/*
 	 * Set up short-lived ParamListInfo
@@ -3626,7 +3626,8 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
 	{
 		ListCell   *l;
 
-		exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK);
+		exec_prepare_plan(estate, expr,
+		  expr->cursor_options | CURSOR_OPT_PARALLEL_OK);
 		stmt->mod_stmt = false;
 		foreach(l, SPI_plan_get_plan_sources(expr->plan))
 		{
@@ -4096,7 +4097,8 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 		 */
 		query = stmt->query;
 		if (query->plan == NULL)
-			exec_prepare_plan(estate, query, stmt->cursor_options);
+			exec_prepare_plan(estate, query,
+			  query->cursor_options | stmt->cursor_options);
 	}
 	else if (stmt->dynquery != NULL)
 	{
@@ -4167,7 +4169,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 
 		query = curvar->cursor_explicit_expr;
 		if (query->plan == NULL)
-			exec_prepare_plan(estate, query, curvar->cursor_options);
+			exec_prepare_plan(estate, query, query->cursor_options);
 	}
 
 	/*
@@ -4366,7 +4368,7 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
 	 */
 	if (expr->plan == NULL)
 	{
-		exec_prepare_plan(estate, expr, 0);
+		exec_prepare_plan(estate, expr, expr->cursor_options);
 		if (target->dtype == PLPGSQL_DTYPE_VAR)
 			exec_check_rw_parameter(expr, target->dno);
 	}
@@ -5174,7 +5176,8 @@ exec_eval_expr(PLpgSQL_execstate *estate,
 	 * If first time through, create a plan for this expression.
 	 */
 	if 

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

[Pavel Stehule]

2017-03-19 14:30 GMT+01:00 Petr Jelinek :

> On 19/03/17 12:32, Pavel Stehule wrote:
> >
> >
> > 2017-03-18 19:30 GMT+01:00 Petr Jelinek  > >:
> >
> > 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  
> > >>  >>>:
> > >>
> > >> 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
>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d356deb9f5..56da4d6163 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -802,6 +802,32 @@ $$ LANGUAGE plpgsql;
 happen in a plain SQL command.

   
+
+  
+   Block level PRAGMA
+
+   
+PRAGMA
+in PL/pgSQL
+   
+
+   
+The block level PRAGMA allows to change some
+PL/pgSQL compiler behave. Currently
+only PRAGMA PLAN_CACHE is supported.
+
+
+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
+  RETURN EXISTS(SELECT * FROM tab WHERE id = _id);
+END;
+$$ LANGUAGE plpgsql;
+
+   
+  
   
 
   
@@ -4649,6 +4675,43 @@ $$ LANGUAGE plpgsql;
  use of the now() function would still be a better idea.
 
 
+
+
+ PRAGMA PLAN_CACHE
+
+ 
+  The plan cache behave can be controlled with PRAGMA 

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

[Petr Jelinek]

On 19/03/17 12:32, Pavel Stehule wrote:
> 
> 
> 2017-03-18 19:30 GMT+01:00 Petr Jelinek  >:
> 
> 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  
> >> >>:
> >>
> >> 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Pavel Stehule]

2017-03-18 19:30 GMT+01:00 Petr Jelinek :

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

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

[Petr Jelinek]

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 > >:
>>
>> 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[David Steele]

On 2/1/17 3:59 PM, Pavel Stehule wrote:
> Hi
> 
> 2017-01-24 21:33 GMT+01:00 Pavel Stehule  >:
> 
> 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
da...@pgmasters.net


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

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

[Jim Nasby]

On 1/27/17 4:14 AM, Greg Stark wrote:

On 25 January 2017 at 20:06, Jim Nasby  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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Pavel Stehule]

Hi

2017-01-24 21:33 GMT+01:00 Pavel Stehule :

>
>
>
>>
>>> 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
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d3272e1..97c59db 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -802,6 +802,32 @@ $$ LANGUAGE plpgsql;
 happen in a plain SQL command.

   
+
+  
+   Block level PRAGMA
+
+   
+PRAGMA
+in PL/pgSQL
+   
+
+   
+The block level PRAGMA allows to change some
+PL/pgSQL compiler behave. Currently
+only PRAGMA PLAN_CACHE is supported.
+
+
+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
+  RETURN EXISTS(SELECT * FROM tab WHERE id = _id);
+END;
+$$ LANGUAGE plpgsql;
+
+   
+  
   
 
   
@@ -4649,6 +4675,43 @@ $$ LANGUAGE plpgsql;
  use of the now() function would still be a better idea.
 
 
+
+
+ PRAGMA PLAN_CACHE
+
+ 
+  The plan cache behave can be controlled with PRAGMA PLAN_CACHE.
+  This PRAGMA can be used on function or on block level (per
+  function, per block). The following options are possible:
+  DEFAULT - default PL/pgSQL
+  implementation - the system try to decide between custom plan and generic
+  plan after five query executions, FORCE_CUSTOM_PLAN
+  - the execution plan is one shot plan - it is specific for every set of
+  used paramaters, FORCE_GENERIC_PLAN - the query plan
+  is generic from start.
+ 
+
+ 
+  
+   PRAGMA PLAN_CACHE
+   in PL/pgSQL
+  
+  The plan for INSERT is generic from begin. The 
+  PRAGMA PLAN_CACHE is related to function - etc. every command
+  in this function will use generic plan.
+
+CREATE FUNCTION logfunc2(logtxt text) RETURNS void AS $$
+PRAGMA PLAN_CACHE(FORCE_GENERIC_PLAN);
+DECLARE
+curtime timestamp;
+BEGIN
+curtime := 'now';
+INSERT INTO logtable VALUES (logtxt, curtime);
+END;
+$$ LANGUAGE plpgsql;
+
+ 
+
   
 
   
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b25b3f1..304fc91 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -51,6 +51,8 @@ bool		plpgsql_check_syntax = false;
 
 PLpgSQL_function *plpgsql_curr_compile;
 
+PLpgSQL_directives *plpgsql_directives;
+
 /* A context appropriate for short-term allocs during compilation */
 MemoryContext plpgsql_compile_tmp_cxt;
 
@@ -83,6 +85,11 @@ static const ExceptionLabelMap exception_label_map[] = {
 	{NULL, 0}
 };
 
+PLpgSQL_directives default_directives = {
+	NULL,
+	true,		/* is_function_scope */
+	0			/* no special cursor option */
+};
 
 /* --
  * static prototypes
@@ -374,6 +381,9 @@ do_compile(FunctionCallInfo fcinfo,
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
+	/* set default compile directives */
+	plpgsql_directives = _directives;
+
 	switch (function->fn_is_trigger)
 	{
 		case PLPGSQL_NOT_TRIGGER:
@@ -852,6 +862,9 @@ plpgsql_compile_inline(char *proc_source)
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
+	/* set default compile directives */
+	plpgsql_directives = _directives;
+
 	/* Set up as though in a function returning VOID */
 	function->fn_rettype = VOIDOID;
 	function->fn_retset = false;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 6fc3db0..0a01bbe 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2335,7 +2335,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
 	Assert(query);
 
 	if (query->plan == NULL)
-		exec_prepare_plan(estate, query, curvar->cursor_options);
+		exec_prepare_plan(estate, query, query->cursor_options);
 
 	/*
 	 * Set up short-lived ParamListInfo
@@ -3625,7 +3625,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
 	{
 		ListCell   *l;
 
-		exec_prepare_plan(estate, expr, 0);
+		exec_prepare_plan(estate, expr, expr->cursor_options);
 		stmt->mod_stmt = false;
 		foreach(l, 

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

[Greg Stark]

On 25 January 2017 at 20:06, Jim Nasby  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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Pavel Stehule]

2017-01-25 21:06 GMT+01:00 Jim Nasby :

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

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

[Jim Nasby]

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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

[Pavel Stehule]

2017-01-24 6:38 GMT+01:00 Pavel Stehule :

> Hi
>
> 2017-01-23 21:59 GMT+01:00 Jim Nasby :
>
>> 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)
>>
>
>
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b25b3f1..304fc91 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -51,6 +51,8 @@ bool		plpgsql_check_syntax = false;
 
 PLpgSQL_function *plpgsql_curr_compile;
 
+PLpgSQL_directives *plpgsql_directives;
+
 /* A context appropriate for short-term allocs during compilation */
 MemoryContext plpgsql_compile_tmp_cxt;
 
@@ -83,6 +85,11 @@ static const ExceptionLabelMap exception_label_map[] = {
 	{NULL, 0}
 };
 
+PLpgSQL_directives default_directives = {
+	NULL,
+	true,		/* is_function_scope */
+	0			/* no special cursor option */
+};
 
 /* --
  * static prototypes
@@ -374,6 +381,9 @@ do_compile(FunctionCallInfo fcinfo,
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
+	/* set default compile directives */
+	plpgsql_directives = _directives;
+
 	switch (function->fn_is_trigger)
 	{
 		case PLPGSQL_NOT_TRIGGER:
@@ -852,6 +862,9 @@ plpgsql_compile_inline(char *proc_source)
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
+	/* set default compile directives */
+	plpgsql_directives = _directives;
+
 	/* Set up as though in a function returning VOID */
 	function->fn_rettype = VOIDOID;
 	function->fn_retset = false;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index b48146a..9971ed2 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2335,7 +2335,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
 	Assert(query);
 
 	if (query->plan == NULL)
-		exec_prepare_plan(estate, query, curvar->cursor_options);
+		exec_prepare_plan(estate, query, query->cursor_options);
 
 	/*
 	 * Set up short-lived ParamListInfo
@@ -3625,7 +3625,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
 	{
 		ListCell   *l;
 
-		exec_prepare_plan(estate, expr, 0);
+		exec_prepare_plan(estate, expr, expr->cursor_options);
 		stmt->mod_stmt = false;
 		foreach(l, SPI_plan_get_plan_sources(expr->plan))
 		{
@@ -4096,7 +4096,8 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 		 */
 		query = stmt->query;
 		if (query->plan == NULL)
-			exec_prepare_plan(estate, query, stmt->cursor_options);
+			exec_prepare_plan(estate, query,
+			  query->cursor_options | stmt->cursor_options);
 	}
 	else if (stmt->dynquery != NULL)
 	{
@@ -4167,7 +4168,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
 
 		query = curvar->cursor_explicit_expr;
 		if (query->plan == NULL)
-			exec_prepare_plan(estate, query, curvar->cursor_options);
+			exec_prepare_plan(estate, query, query->cursor_options);
 	}
 
 	/*
@@ -4366,7 +4367,7 @@ exec_assign_expr(PLpgSQL_execstate 

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

[Pavel Stehule]

2017-01-23 21:59 GMT+01:00 Jim Nasby :

> 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

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

[Pavel Stehule]

Hi

2017-01-23 21:59 GMT+01:00 Jim Nasby :

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

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

[Jim Nasby]

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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] 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
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index b25b3f1..304fc91 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -51,6 +51,8 @@ bool		plpgsql_check_syntax = false;
 
 PLpgSQL_function *plpgsql_curr_compile;
 
+PLpgSQL_directives *plpgsql_directives;
+
 /* A context appropriate for short-term allocs during compilation */
 MemoryContext plpgsql_compile_tmp_cxt;
 
@@ -83,6 +85,11 @@ static const ExceptionLabelMap exception_label_map[] = {
 	{NULL, 0}
 };
 
+PLpgSQL_directives default_directives = {
+	NULL,
+	true,		/* is_function_scope */
+	0			/* no special cursor option */
+};
 
 /* --
  * static prototypes
@@ -374,6 +381,9 @@ do_compile(FunctionCallInfo fcinfo,
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
+	/* set default compile directives */
+	plpgsql_directives = _directives;
+
 	switch (function->fn_is_trigger)
 	{
 		case PLPGSQL_NOT_TRIGGER:
@@ -852,6 +862,9 @@ plpgsql_compile_inline(char *proc_source)
 	plpgsql_DumpExecTree = false;
 	plpgsql_start_datums();
 
+	/* set default compile directives */
+	plpgsql_directives = _directives;
+
 	/* Set up as though in a function returning VOID */
 	function->fn_rettype = VOIDOID;
 	function->fn_retset = false;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index b48146a..66b3ce9 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3625,7 +3625,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
 	{
 		ListCell   *l;
 
-		exec_prepare_plan(estate, expr, 0);
+		exec_prepare_plan(estate, expr, expr->cursor_options);
 		stmt->mod_stmt = false;
 		foreach(l, SPI_plan_get_plan_sources(expr->plan))
 		{
@@ -4366,7 +4366,7 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
 	 */
 	if (expr->plan == NULL)
 	{
-		exec_prepare_plan(estate, expr, 0);
+		exec_prepare_plan(estate, expr, expr->cursor_options);
 		if (target->dtype == PLPGSQL_DTYPE_VAR)
 			exec_check_rw_parameter(expr, target->dno);
 	}
@@ -5173,7 +5173,7 @@ exec_eval_expr(PLpgSQL_execstate *estate,
 	 * If first time through, create a plan for this expression.
 	 */
 	if (expr->plan == NULL)
-		exec_prepare_plan(estate, expr, 0);
+		exec_prepare_plan(estate, expr, expr->cursor_options);
 
 	/*
 	 * If this is a simple expression, bypass SPI and use the executor
@@ -5252,8 +5252,8 @@ exec_run_select(PLpgSQL_execstate *estate,
 	 * On the first call for this expression generate the plan
 	 */
 	if (expr->plan == NULL)
-		exec_prepare_plan(estate, expr, parallelOK ?
-		  CURSOR_OPT_PARALLEL_OK : 0);
+		exec_prepare_plan(estate, expr, (parallelOK ?
+		  CURSOR_OPT_PARALLEL_OK : 0) | expr->cursor_options);
 
 	/*
 	 * If a portal was requested, put the query into the portal
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 906fe01..65fea0e 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -1535,6 +1535,10 @@ static void
 dump_expr(PLpgSQL_expr *expr)
 {
 	printf("'%s'", expr->query);
+	if (expr->cursor_options & CURSOR_OPT_GENERIC_PLAN)
+		printf("/* GENERIC_PLAN! */");
+	if (expr->cursor_options & CURSOR_OPT_CUSTOM_PLAN)
+		printf("/* CUSTOM_PLAN! */");
 }
 
 void
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 4a4cd6a..3f236b5 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -108,6 +108,9 @@ static	PLpgSQL_expr