proposal - plpgsql - all plpgsql auto variables should be constant

Started by Pavel Stehuleover 5 years ago9 messages

pavel.stehule@gmail.com

over 5 years ago

1 attachment(s)

plpgsql generate lot of auto variables - FOUND, SQLERRM, cycle's control
variable, TG_WHEN, TG_OP, ..

Currently these variables are not protected, what can be source of
problems, mainly for not experienced users. I propose mark these variables
as constant.

-- today
postgres=# do $$ begin for i in 1..10 loop raise notice 'i=%', i; i := 20;
end loop; end; $$;
NOTICE: i=1
NOTICE: i=2
NOTICE: i=3
NOTICE: i=4
NOTICE: i=5
NOTICE: i=6
NOTICE: i=7
NOTICE: i=8
NOTICE: i=9
NOTICE: i=10
DO

-- after patch
postgres=# do $$ begin for i in 1..10 loop raise notice 'i=%', i; i := 20;
end loop; end; $$;
ERROR: variable "i" is declared CONSTANT
LINE 1: ... begin for i in 1..10 loop raise notice 'i=%', i; i := 20; e...

These variables are protected in PL/SQL too.

Comments, notes?

Regards

Pavel

p.s. this is simple implementation - just for function demo. Maybe can be
better to introduce new plpgsql_variable's flag like is_protected or
similar than using isconst.

Attachments:

plpgsql-make-auto-const.patchtext/x-patch; charset=US-ASCII; name=plpgsql-make-auto-const.patchDownload

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 828ff5a288..a9b597810b 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -443,7 +443,7 @@ do_compile(FunctionCallInfo fcinfo,
 				argvariable = plpgsql_build_variable((argnames &&
 													  argnames[i][0] != '\0') ?
 													 argnames[i] : buf,
-													 0, argdtype, false);
+													 0, argdtype, false, false);
 
 				if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
 				{
@@ -579,7 +579,7 @@ do_compile(FunctionCallInfo fcinfo,
 															 -1,
 															 function->fn_input_collation,
 															 NULL),
-											  true);
+											  true, true);
 			}
 
 			ReleaseSysCache(typeTup);
@@ -614,7 +614,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_NAME;
@@ -625,7 +625,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_WHEN;
@@ -636,7 +636,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_LEVEL;
@@ -647,7 +647,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_OP;
@@ -658,7 +658,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																InvalidOid,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_RELID;
@@ -669,7 +669,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_TABLE_NAME;
@@ -680,7 +680,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_TABLE_NAME;
@@ -691,7 +691,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_TABLE_SCHEMA;
@@ -702,7 +702,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																InvalidOid,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_NARGS;
@@ -713,7 +713,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_ARGV;
@@ -739,7 +739,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_EVENT;
@@ -750,7 +750,7 @@ do_compile(FunctionCallInfo fcinfo,
 																-1,
 																function->fn_input_collation,
 																NULL),
-										 true);
+										 true, true);
 			Assert(var->dtype == PLPGSQL_DTYPE_VAR);
 			var->dtype = PLPGSQL_DTYPE_PROMISE;
 			((PLpgSQL_var *) var)->promise = PLPGSQL_PROMISE_TG_TAG;
@@ -774,7 +774,7 @@ do_compile(FunctionCallInfo fcinfo,
 														-1,
 														InvalidOid,
 														NULL),
-								 true);
+								 true, true);
 	function->found_varno = var->dno;
 
 	/*
@@ -929,7 +929,7 @@ plpgsql_compile_inline(char *proc_source)
 														-1,
 														InvalidOid,
 														NULL),
-								 true);
+								 true, true);
 	function->found_varno = var->dno;
 
 	/*
@@ -1843,7 +1843,7 @@ plpgsql_parse_cwordrowtype(List *idents)
  */
 PLpgSQL_variable *
 plpgsql_build_variable(const char *refname, int lineno, PLpgSQL_type *dtype,
-					   bool add2namespace)
+					   bool add2namespace, bool isconst)
 {
 	PLpgSQL_variable *result;
 
@@ -1898,6 +1898,8 @@ plpgsql_build_variable(const char *refname, int lineno, PLpgSQL_type *dtype,
 			break;
 	}
 
+	result->isconst = isconst;
+
 	return result;
 }
 
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 6778d0e771..89a4c07e19 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -513,8 +513,7 @@ decl_statement	: decl_varname decl_const decl_datatype decl_collate decl_notnull
 						}
 
 						var = plpgsql_build_variable($1.name, $1.lineno,
-													 $3, true);
-						var->isconst = $2;
+													 $3, true, $2);
 						var->notnull = $5;
 						var->default_val = $6;
 
@@ -553,7 +552,7 @@ decl_statement	: decl_varname decl_const decl_datatype decl_collate decl_notnull
 																		  -1,
 																		  InvalidOid,
 																		  NULL),
-												   true);
+												   true, false);
 
 						curname_def = palloc0(sizeof(PLpgSQL_expr));
 
@@ -655,7 +654,7 @@ decl_cursor_arg : decl_varname decl_datatype
 					{
 						$$ = (PLpgSQL_datum *)
 							plpgsql_build_variable($1.name, $1.lineno,
-												   $2, true);
+												   $2, true, false);
 					}
 				;
 
@@ -1507,7 +1506,7 @@ for_control		: for_variable K_IN
 																				  -1,
 																				  InvalidOid,
 																				  NULL),
-														   true);
+														   true, true);
 
 								new = palloc0(sizeof(PLpgSQL_stmt_fori));
 								new->cmd_type = PLPGSQL_STMT_FORI;
@@ -2321,7 +2320,7 @@ exception_sect	:
 																			-1,
 																			plpgsql_curr_compile->fn_input_collation,
 																			NULL),
-													 true);
+													 true, true);
 						var->isconst = true;
 						new->sqlstate_varno = var->dno;
 
@@ -2330,7 +2329,7 @@ exception_sect	:
 																			-1,
 																			plpgsql_curr_compile->fn_input_collation,
 																			NULL),
-													 true);
+													 true, true);
 						var->isconst = true;
 						new->sqlerrm_varno = var->dno;
 
@@ -4089,7 +4088,7 @@ make_case(int location, PLpgSQL_expr *t_expr,
 														  -1,
 														  InvalidOid,
 														  NULL),
-								   true);
+								   true, true);
 		new->t_varno = t_var->dno;
 
 		foreach(l, case_when_list)
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index 1af2595e34..bfeca5dcf0 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1254,7 +1254,8 @@ extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod,
 											TypeName *origtypname);
 extern PLpgSQL_variable *plpgsql_build_variable(const char *refname, int lineno,
 												PLpgSQL_type *dtype,
-												bool add2namespace);
+												bool add2namespace,
+												bool isconst);
 extern PLpgSQL_rec *plpgsql_build_record(const char *refname, int lineno,
 										 PLpgSQL_type *dtype, Oid rectypeid,
 										 bool add2namespace);

Ashutosh Bapat

ashutosh.bapat.oss@gmail.com

over 5 years ago

In reply to: Pavel Stehule (#1)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

On Fri, Apr 24, 2020 at 12:24 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:

Hi

plpgsql generate lot of auto variables - FOUND, SQLERRM, cycle's control variable, TG_WHEN, TG_OP, ..

Currently these variables are not protected, what can be source of problems, mainly for not experienced users. I propose mark these variables as constant.

+1 for general idea.

-- today
postgres=# do $$ begin for i in 1..10 loop raise notice 'i=%', i; i := 20; end loop; end; $$;
NOTICE: i=1
NOTICE: i=2
NOTICE: i=3
NOTICE: i=4
NOTICE: i=5
NOTICE: i=6
NOTICE: i=7
NOTICE: i=8
NOTICE: i=9
NOTICE: i=10
DO

-- after patch
postgres=# do $$ begin for i in 1..10 loop raise notice 'i=%', i; i := 20; end loop; end; $$;
ERROR: variable "i" is declared CONSTANT

CONSTANT looks odd in this context since i's value changes. But you
already have a proposal to change that.

p.s. this is simple implementation - just for function demo. Maybe can be better to introduce new plpgsql_variable's flag like is_protected or similar than using isconst.

Yes, I think that will help. In this case PL/SQL says that "i" can not
be used as an assignment target. That's not very clear but something
on those lines will help.

--
Best Wishes,
Ashutosh Bapat

Tom Lane

tgl@sss.pgh.pa.us

over 5 years ago

In reply to: Ashutosh Bapat (#2)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

On Fri, Apr 24, 2020 at 12:24 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:

plpgsql generate lot of auto variables - FOUND, SQLERRM, cycle's control variable, TG_WHEN, TG_OP, ..
Currently these variables are not protected, what can be source of problems, mainly for not experienced users. I propose mark these variables as constant.

+1 for general idea.

I'm skeptical. If we'd marked them that way from day one, it would have
been fine, but to change it now is a whole different discussion. I think
the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain that
there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their code.

As for the trigger variables specifically, what is the rationale
for marking TG_OP read-only but not OLD and NEW? But it is dead
certain that we won't get away with making the latter two read-only.

In short, -1. This ship sailed about twenty years ago.

regards, tom lane

Pavel Stehule

pavel.stehule@gmail.com

over 5 years ago

In reply to: Tom Lane (#3)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

pá 24. 4. 2020 v 16:07 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

On Fri, Apr 24, 2020 at 12:24 PM Pavel Stehule <pavel.stehule@gmail.com>

wrote:

plpgsql generate lot of auto variables - FOUND, SQLERRM, cycle's

control variable, TG_WHEN, TG_OP, ..

Currently these variables are not protected, what can be source of

problems, mainly for not experienced users. I propose mark these variables
as constant.

+1 for general idea.

I'm skeptical. If we'd marked them that way from day one, it would have
been fine, but to change it now is a whole different discussion. I think
the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain that
there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their code.

This is not black/white issue. Maybe can sense to modify the FOUND
variable, but modification of control variable has not any sense. The
updated value is rewriten by runtime any iteration. You cannot to use
modification of control variable to skip some iterations like in C.

As for the trigger variables specifically, what is the rationale
for marking TG_OP read-only but not OLD and NEW? But it is dead
certain that we won't get away with making the latter two read-only.

For before triggers the NEW have to be updated. Any other maybe should be
protected, but there is little bit different kind of informations.

Show quoted text

In short, -1. This ship sailed about twenty years ago.

regards, tom lane

Kyotaro Horiguchi

horikyota.ntt@gmail.com

over 5 years ago

In reply to: Pavel Stehule (#4)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

At Fri, 24 Apr 2020 16:47:28 +0200, Pavel Stehule <pavel.stehule@gmail.com> wrote in

pá 24. 4. 2020 v 16:07 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

On Fri, Apr 24, 2020 at 12:24 PM Pavel Stehule <pavel.stehule@gmail.com>

wrote:

plpgsql generate lot of auto variables - FOUND, SQLERRM, cycle's

control variable, TG_WHEN, TG_OP, ..

Currently these variables are not protected, what can be source of

problems, mainly for not experienced users. I propose mark these variables
as constant.

+1 for general idea.

I'm skeptical. If we'd marked them that way from day one, it would have
been fine, but to change it now is a whole different discussion. I think
the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain that
there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their code.

This is not black/white issue. Maybe can sense to modify the FOUND
variable, but modification of control variable has not any sense. The
updated value is rewriten by runtime any iteration. You cannot to use
modification of control variable to skip some iterations like in C.

It seems to me, the loop structure is not a parallel of for() in C. It
is rather a parallel of foreach of Perl or "for in range()" in
Python. So it is natural to me that the i is assignable and reset with
the next value at every iteration. I believe that there are many
existing cases where the control variable is modified in a loop.

On the other hand, I'm not sure about FOUND and the similars and I
don't have a firm opinion them. I don't see a use case where they need
to be assignable. However, I don't see a clear reason they mustn't be
assignable, too. (And the behavior is documented at least for FOUND.)

As for the trigger variables specifically, what is the rationale
for marking TG_OP read-only but not OLD and NEW? But it is dead
certain that we won't get away with making the latter two read-only.

For before triggers the NEW have to be updated. Any other maybe should be
protected, but there is little bit different kind of informations.

In short, -1. This ship sailed about twenty years ago.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Pavel Stehule

pavel.stehule@gmail.com

over 5 years ago

In reply to: Kyotaro Horiguchi (#5)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

po 27. 4. 2020 v 5:02 odesílatel Kyotaro Horiguchi <horikyota.ntt@gmail.com>
napsal:

At Fri, 24 Apr 2020 16:47:28 +0200, Pavel Stehule <pavel.stehule@gmail.com>
wrote in

pá 24. 4. 2020 v 16:07 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:

On Fri, Apr 24, 2020 at 12:24 PM Pavel Stehule <

pavel.stehule@gmail.com>

wrote:

plpgsql generate lot of auto variables - FOUND, SQLERRM, cycle's

control variable, TG_WHEN, TG_OP, ..

Currently these variables are not protected, what can be source of

problems, mainly for not experienced users. I propose mark these

variables

as constant.

+1 for general idea.

I'm skeptical. If we'd marked them that way from day one, it would

have

been fine, but to change it now is a whole different discussion. I

think

the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain

that

there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their

code.

This is not black/white issue. Maybe can sense to modify the FOUND
variable, but modification of control variable has not any sense. The
updated value is rewriten by runtime any iteration. You cannot to use
modification of control variable to skip some iterations like in C.

It seems to me, the loop structure is not a parallel of for() in C. It
is rather a parallel of foreach of Perl or "for in range()" in
Python. So it is natural to me that the i is assignable and reset with
the next value at every iteration. I believe that there are many
existing cases where the control variable is modified in a loop.

it is based on PL/SQL language and this language is based on ADA.

There loop parameter is constant

https://www.adaic.org/resources/add_content/standards/05aarm/html/AA-5-5.html

Regards

Pavel

Show quoted text

On the other hand, I'm not sure about FOUND and the similars and I
don't have a firm opinion them. I don't see a use case where they need
to be assignable. However, I don't see a clear reason they mustn't be
assignable, too. (And the behavior is documented at least for FOUND.)

As for the trigger variables specifically, what is the rationale
for marking TG_OP read-only but not OLD and NEW? But it is dead
certain that we won't get away with making the latter two read-only.

For before triggers the NEW have to be updated. Any other maybe should be
protected, but there is little bit different kind of informations.

In short, -1. This ship sailed about twenty years ago.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Pavel Stehule

pavel.stehule@gmail.com

over 5 years ago

In reply to: Pavel Stehule (#1)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

po 27. 4. 2020 v 16:26 odesílatel Greg Stark <stark@mit.edu> napsal:

On Fri, 24 Apr 2020 at 10:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm skeptical. If we'd marked them that way from day one, it would have
been fine, but to change it now is a whole different discussion. I think
the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain that
there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their code.

I kind of doubt it would break anybody's code. But I also doubt it's
actually going to help anybody. It's not exactly an easy bug to write,
so meh, I can't really get worked up either way about this.

As for the trigger variables specifically, what is the rationale
for marking TG_OP read-only but not OLD and NEW? But it is dead
certain that we won't get away with making the latter two read-only.

But, uh, this actually seems like it might help people. Obviously we
can't make NEW constant for BEFORE triggers, but for AFTER triggers it
would actually be catching quite an easy-to-write bug. I bet plenty of
people accidentally define triggers as AFTER triggers which are
intending to modify the columns being stored and then don't understand
why they aren't working.

They might not even find out right away if the trigger only modifies
the columns sometimes so it could be the kind of latent bug that
catches people in production (which wouldn't be improved by the patch
but at least it would produce an error rather than silent data
corruption).

The only valid use cases that maybe would cause some pain would be
people using the same function for BEFORE *and* AFTER triggers and
where that code is written to just assign to NEW in both cases. That
seems like it would be odd though since we're not talking about an
audit function or something like that if the function is assigning to
NEW...

this behave can be dynamic and it can be active only for AFTER trigger

Show quoted text

--
greg

Import Notes

Reply to msg id not found: CAM-w4HMDx_pGcqzrMOY3KvFz+ECi0+gpuMZa7M8BGV5fU26okw@mail.gmail.com

Ashutosh Bapat

ashutosh.bapat.oss@gmail.com

over 5 years ago

In reply to: Pavel Stehule (#1)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

On Mon, Apr 27, 2020 at 7:56 PM Greg Stark <stark@mit.edu> wrote:

On Fri, 24 Apr 2020 at 10:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm skeptical. If we'd marked them that way from day one, it would have
been fine, but to change it now is a whole different discussion. I think
the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain that
there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their code.

I kind of doubt it would break anybody's code. But I also doubt it's
actually going to help anybody. It's not exactly an easy bug to write,
so meh, I can't really get worked up either way about this.

We could retain the old behaviour by using a GUC which defaults to old
behaviour. More GUCs means more confusion, this once guc under plpgsql
extension might actually help.

--
Best Wishes,
Ashutosh Bapat

Import Notes

Reply to msg id not found: CAM-w4HMDx_pGcqzrMOY3KvFz+ECi0+gpuMZa7M8BGV5fU26okw@mail.gmail.com

Pavel Stehule

pavel.stehule@gmail.com

over 5 years ago

In reply to: Ashutosh Bapat (#8)

Re: proposal - plpgsql - all plpgsql auto variables should be constant

út 28. 4. 2020 v 13:35 odesílatel Ashutosh Bapat <
ashutosh.bapat.oss@gmail.com> napsal:

On Mon, Apr 27, 2020 at 7:56 PM Greg Stark <stark@mit.edu> wrote:

On Fri, 24 Apr 2020 at 10:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm skeptical. If we'd marked them that way from day one, it would

have

been fine, but to change it now is a whole different discussion. I

think

the odds that anybody will thank us are much smaller than the odds that
there will be complaints. In particular, I'd be just about certain

that

there are people out there who are changing FOUND and loop control
variables manually, and they will not appreciate us breaking their

code.

I kind of doubt it would break anybody's code. But I also doubt it's
actually going to help anybody. It's not exactly an easy bug to write,
so meh, I can't really get worked up either way about this.

We could retain the old behaviour by using a GUC which defaults to old
behaviour. More GUCs means more confusion, this once guc under plpgsql
extension might actually help.

I am not sure if other GUC can help (in this case). Probably it cannot be
default, and beginners has zero knowledge to enable this or similar GUC.

This week I enhanced plpgsql_check about new check
https://github.com/okbob/plpgsql_check related to this feature.

I afraid so people who needs these checks and some help probably doesn't
know about this extension.

Show quoted text

--
Best Wishes,
Ashutosh Bapat