Re: [HACKERS] Clobbered parameter names via DECLARE in PL/PgSQL
Hello there is VIP patch of plpgsql_check_function that supports this warning Regards Pavel 2012/4/15 Pavel Stehule pavel.steh...@gmail.com: 2012/4/15 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: We can raise warning from CREATE OR REPLACE FUNCTION - but I would to like have plpgsql_check_function inside core - and it is better place for this and similar issues. I agree. This is a perfectly legal use of nested declaration scopes, so it would be totally inappropriate to complain about it in normal use of a plpgsql function. On the other hand, it would probably be sane and useful for CHECK FUNCTION to flag any case where an inner declaration shadows an outer-scope name (not only the specific case of topmost block vs function parameter). yes, it is very simple check there. There should be levels of warnings in future and performance or semantic warnings. But, we don't need to increase complexity of CHECK FUNCTION now. A design of CHECK FUNCTION was rich for this purposes. And we need to find way to push plpgsql_check_function to core first. Regards Pavel regards, tom lane plpgsql_check_function-2012-04-17-1.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Clobbered parameter names via DECLARE in PL/PgSQL
Hello hackers, It turns out that in a PL/PgSQL function, you can DECLARE a variable using the same name as one of the function parameters. This has the effect of clobbering the parameter, for example: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE foo text; BEGIN RETURN foo; END; $$; SELECT declare_clobber(1); == NULL On the other hand, PL/PgSQL does protect against duplicate definitions within DECLARE: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE foo int; foo text; BEGIN RETURN foo; END; $$; == ERROR: duplicate declaration at or near foo And it also protects against using a DECLAREd name as a parameter alias: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE bar int; bar ALIAS FOR $1; BEGIN RETURN bar; END; $$; == ERROR: duplicate declaration at or near bar I would suggest that if the user DECLAREs a variable with the same name as a parameter, it is very evidently a programming error, and we should raise the same duplicate declaration error. I haven't yet looked at how difficult this would be to fix, but if there are no objections I would like to attempt a patch. Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
2012/4/15 Brendan Jurd dire...@gmail.com: Hello hackers, It turns out that in a PL/PgSQL function, you can DECLARE a variable using the same name as one of the function parameters. This has the effect of clobbering the parameter, for example: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE foo text; BEGIN RETURN foo; END; $$; SELECT declare_clobber(1); == NULL On the other hand, PL/PgSQL does protect against duplicate definitions within DECLARE: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE foo int; foo text; BEGIN RETURN foo; END; $$; == ERROR: duplicate declaration at or near foo And it also protects against using a DECLAREd name as a parameter alias: CREATE OR REPLACE FUNCTION declare_clobber(foo int) RETURNS int LANGUAGE plpgsql AS $$ DECLARE bar int; bar ALIAS FOR $1; BEGIN RETURN bar; END; $$; == ERROR: duplicate declaration at or near bar I would suggest that if the user DECLAREs a variable with the same name as a parameter, it is very evidently a programming error, and we should raise the same duplicate declaration error. I haven't yet looked at how difficult this would be to fix, but if there are no objections I would like to attempt a patch. I disagree - variables and parameters are in different namespace so you can exactly identify variable and parameter. More - it is compatibility break. If plpgsql_check_function exists, then this check can be implemented as warning. Regards Pavel Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
On 15 April 2012 17:55, Pavel Stehule pavel.steh...@gmail.com wrote: 2012/4/15 Brendan Jurd dire...@gmail.com: It turns out that in a PL/PgSQL function, you can DECLARE a variable using the same name as one of the function parameters. This has the effect of clobbering the parameter, for example: ... I would suggest that if the user DECLAREs a variable with the same name as a parameter, it is very evidently a programming error, and we should raise the same duplicate declaration error. I haven't yet looked at how difficult this would be to fix, but if there are no objections I would like to attempt a patch. I disagree - variables and parameters are in different namespace so you can exactly identify variable and parameter. More - it is compatibility break. They may technically be in different namespaces, but the fact that the declared variable quietly goes ahead and masks the parameter locally, seems like a recipe for unexpected consequences. It certainly was in my case, and I doubt I'm the first or the last to make that mistake. Under these conditions, you now have foo which refers to the variable, and declare_clobber.foo which refers to the parameter. Not exactly a model of clarity, and it's also quite easy to miss the part of the PL/PgSQL docs mentioning this notation. Perhaps it's a failure of imagination on my part, but I can't think of a legitimate reason for a programmer to deliberately use the same name to refer to a declared variable and a function parameter. What would be the benefit? Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
2012/4/15 Brendan Jurd dire...@gmail.com: On 15 April 2012 17:55, Pavel Stehule pavel.steh...@gmail.com wrote: 2012/4/15 Brendan Jurd dire...@gmail.com: It turns out that in a PL/PgSQL function, you can DECLARE a variable using the same name as one of the function parameters. This has the effect of clobbering the parameter, for example: ... I would suggest that if the user DECLAREs a variable with the same name as a parameter, it is very evidently a programming error, and we should raise the same duplicate declaration error. I haven't yet looked at how difficult this would be to fix, but if there are no objections I would like to attempt a patch. I disagree - variables and parameters are in different namespace so you can exactly identify variable and parameter. More - it is compatibility break. They may technically be in different namespaces, but the fact that the declared variable quietly goes ahead and masks the parameter locally, seems like a recipe for unexpected consequences. It certainly was in my case, and I doubt I'm the first or the last to make that mistake. I agree so this issue is relative usual. But I don't think so we can use too hard solution like exclusion of parameter names, because it just has not support in standard or PL/SQL or SQL/PSM. And we introduce a few years ago different solution - function's namespace. Under these conditions, you now have foo which refers to the variable, and declare_clobber.foo which refers to the parameter. Not exactly a model of clarity, and it's also quite easy to miss the part of the PL/PgSQL docs mentioning this notation. it is not well documented and should be documented better: http://www.postgresql.org/docs/9.1/static/plpgsql-structure.html 39.2. Structure of PL/pgSQL Note: There is actually a hidden outer block surrounding the body of any PL/pgSQL function. This block provides the declarations of the function's parameters (if any), as well as some special variables such as FOUND (see Section 39.5.5). The outer block is labeled with the function's name, meaning that parameters and special variables can be qualified with the function's name. 39.3.1. Declaring Function Parameters Note: These two examples are not perfectly equivalent. In the first case, subtotal could be referenced as sales_tax.subtotal, but in the second case it could not. (Had we attached a label to the inner block, subtotal could be qualified with that label, instead. It should be documented better. Perhaps it's a failure of imagination on my part, but I can't think of a legitimate reason for a programmer to deliberately use the same name to refer to a declared variable and a function parameter. What would be the benefit? it depends on level of nesting blocks. For simple functions there parameter redeclaration is clean bug, but for more nested blocks and complex procedures, there should be interesting using some local variables with same identifier like some parameters and blocking parameter's identifier can be same unfriendly feature like RO parameters in previous pg versions. I understand your motivation well, but solution should be warning, not blocking. I think. Regards Pavel Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
On 15 April 2012 18:54, Pavel Stehule pavel.steh...@gmail.com wrote: 2012/4/15 Brendan Jurd dire...@gmail.com: Perhaps it's a failure of imagination on my part, but I can't think of a legitimate reason for a programmer to deliberately use the same name to refer to a declared variable and a function parameter. What would be the benefit? it depends on level of nesting blocks. For simple functions there parameter redeclaration is clean bug, but for more nested blocks and complex procedures, there should be interesting using some local variables with same identifier like some parameters and blocking parameter's identifier can be same unfriendly feature like RO parameters in previous pg versions. I understand your motivation well, but solution should be warning, not blocking. I think. I can accept that ... but I wonder about the implementation of such a warning. Can we raise a WARNING message on CREATE [OR REPLACE] FUNCTION? If so, should there be a way to switch it off? If so, would this be implemented globally, or per-function? Would it be a postgres run-time setting, or an extension to CREATE FUNCTION syntax, or something within the PL/pgSQL code (like Perl's 'use strict')? Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
2012/4/15 Brendan Jurd dire...@gmail.com: On 15 April 2012 18:54, Pavel Stehule pavel.steh...@gmail.com wrote: 2012/4/15 Brendan Jurd dire...@gmail.com: Perhaps it's a failure of imagination on my part, but I can't think of a legitimate reason for a programmer to deliberately use the same name to refer to a declared variable and a function parameter. What would be the benefit? it depends on level of nesting blocks. For simple functions there parameter redeclaration is clean bug, but for more nested blocks and complex procedures, there should be interesting using some local variables with same identifier like some parameters and blocking parameter's identifier can be same unfriendly feature like RO parameters in previous pg versions. I understand your motivation well, but solution should be warning, not blocking. I think. I can accept that ... but I wonder about the implementation of such a warning. Can we raise a WARNING message on CREATE [OR REPLACE] FUNCTION? If so, should there be a way to switch it off? If so, would this be implemented globally, or per-function? Would it be a postgres run-time setting, or an extension to CREATE FUNCTION syntax, or something within the PL/pgSQL code (like Perl's 'use strict')? We can raise warning from CREATE OR REPLACE FUNCTION - but I would to like have plpgsql_check_function inside core - and it is better place for this and similar issues. Now we talk about features in 9.3, and there check_function should be. Regards Pavel Cheers, BJ -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
On 04/15/2012 03:49 AM, Brendan Jurd wrote: Hello hackers, It turns out that in a PL/PgSQL function, you can DECLARE a variable using the same name as one of the function parameters. This has the effect of clobbering the parameter, for example: ... I would suggest that if the user DECLAREs a variable with the same name as a parameter, it is very evidently a programming error, and we should raise the same duplicate declaration error. I haven't yet looked at how difficult this would be to fix, but if there are no objections I would like to attempt a patch. This doesn't need fixing, IMNSHO. The name isn't clobbered and the partameter is accessible. The docs state: Any statement in the statement section of a block can be a subblock. Subblocks can be used for logical grouping or to localize variables to a small group of statements. Variables declared in a subblock mask any similarly-named variables of outer blocks for the duration of the subblock; but you can access the outer variables anyway if you qualify their names with their block's label. ... There is actually a hidden outer block surrounding the body of any PL/pgSQL function. This block provides the declarations of the function's parameters (if any), as well as some special variables such as FOUND (see Section 39.5.5). The outer block is labeled with the function's name, meaning that parameters and special variables can be qualified with the function's name. Note that you can label the outermost block of the function body, as the example in the docs shows. cheers andrew -- 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] Clobbered parameter names via DECLARE in PL/PgSQL
Pavel Stehule pavel.steh...@gmail.com writes: We can raise warning from CREATE OR REPLACE FUNCTION - but I would to like have plpgsql_check_function inside core - and it is better place for this and similar issues. I agree. This is a perfectly legal use of nested declaration scopes, so it would be totally inappropriate to complain about it in normal use of a plpgsql function. On the other hand, it would probably be sane and useful for CHECK FUNCTION to flag any case where an inner declaration shadows an outer-scope name (not only the specific case of topmost block vs function parameter). 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] Clobbered parameter names via DECLARE in PL/PgSQL
2012/4/15 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: We can raise warning from CREATE OR REPLACE FUNCTION - but I would to like have plpgsql_check_function inside core - and it is better place for this and similar issues. I agree. This is a perfectly legal use of nested declaration scopes, so it would be totally inappropriate to complain about it in normal use of a plpgsql function. On the other hand, it would probably be sane and useful for CHECK FUNCTION to flag any case where an inner declaration shadows an outer-scope name (not only the specific case of topmost block vs function parameter). yes, it is very simple check there. There should be levels of warnings in future and performance or semantic warnings. But, we don't need to increase complexity of CHECK FUNCTION now. A design of CHECK FUNCTION was rich for this purposes. And we need to find way to push plpgsql_check_function to core first. Regards Pavel 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
