Thread: [patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
[patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
From
"David G. Johnston"
Date:
Hackers,
Bug # 16519 [1] is another report of confusion regarding trying to use parameters in improper locations - specifically the SET ROLE command within pl/pgsql. I'm re-attaching the doc patch and am adding it to the commitfest.
David J.
Attachment
Re: [patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
From
Pavel Stehule
Date:
čt 26. 11. 2020 v 6:41 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:
Hackers,Bug # 16519 [1] is another report of confusion regarding trying to use parameters in improper locations - specifically the SET ROLE command within pl/pgsql. I'm re-attaching the doc patch and am adding it to the commitfest.
I checked this patch, and I think so it is correct - my comments are just about enhancing by some examples
Maybe for following sentence the some examples can be practical
+ If the SQL command being executed is incapable of returning a result
+ it does not accept query parameters.
</para>
+ it does not accept query parameters.
</para>
+ it does not accept query parameters (usually DDL commands like CREATE TABLE, DROP TABLE, ALTER ... ).
+ Query parameters will only be substituted in places where syntactically allowed
+ (in particular, identifiers can never be replaced with a query parameter.)
+ As an extreme case, consider this example of poor programming style:
+ (in particular, identifiers can never be replaced with a query parameter.)
+ As an extreme case, consider this example of poor programming style:
In this case, I miss the more precious specification of identifiers
+ (in particular, SQL identifiers (like schema, table, column names) can never be replaced with a query parameter.)
Regards
Pavel
Re: [patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
From
"David G. Johnston"
Date:
On Thu, Nov 26, 2020 at 12:49 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
čt 26. 11. 2020 v 6:41 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:Hackers,Bug # 16519 [1] is another report of confusion regarding trying to use parameters in improper locations - specifically the SET ROLE command within pl/pgsql. I'm re-attaching the doc patch and am adding it to the commitfest.I checked this patch, and I think so it is correct - my comments are just about enhancing by some examples
Thank you for the review.
v2 attached.
I added examples in the two places you noted.
Upon re-reading, I decided that opening up the section by including everything then fitting in parameters with an exception for utility commands (without previously/otherwise identifying them) forced some undesirable verbosity. Instead, I opened up with the utility commands as the main body of non-result returning commands and then moved onto delete/insert/update non-returning cases when the subsequent paragraph regarding parameters can then refer to the second class (by way of excluding the first class). This seems to flow better, IMO.
David J.
Attachment
Re: [patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
From
Pavel Stehule
Date:
po 30. 11. 2020 v 4:24 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:
On Thu, Nov 26, 2020 at 12:49 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:čt 26. 11. 2020 v 6:41 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:Hackers,Bug # 16519 [1] is another report of confusion regarding trying to use parameters in improper locations - specifically the SET ROLE command within pl/pgsql. I'm re-attaching the doc patch and am adding it to the commitfest.I checked this patch, and I think so it is correct - my comments are just about enhancing by some examplesThank you for the review.v2 attached.I added examples in the two places you noted.Upon re-reading, I decided that opening up the section by including everything then fitting in parameters with an exception for utility commands (without previously/otherwise identifying them) forced some undesirable verbosity. Instead, I opened up with the utility commands as the main body of non-result returning commands and then moved onto delete/insert/update non-returning cases when the subsequent paragraph regarding parameters can then refer to the second class (by way of excluding the first class). This seems to flow better, IMO.
I have no objections, but maybe these pages are a little bit unclear generally, because the core of the problem is not described.
Personally I miss a description of the reason why variables cannot be used - the description "variables cannot be used in statements without result" is true, but it is not core information.
The important fact is usage of an execution plan or not. The statements with an execution plan can be parametrized (DML - INSERT, UPDATE, DELETE), and SELECT. The statements without execution plans should not be parametrized. The only execution via execution plan executor allows parametrization. DDL statements are executed via utility execution, and their parameterization is not supported.
Regards
Pavel
David J.
Re: [patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
From
"David G. Johnston"
Date:
On Mon, Nov 30, 2020 at 12:51 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:
po 30. 11. 2020 v 4:24 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:On Thu, Nov 26, 2020 at 12:49 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:čt 26. 11. 2020 v 6:41 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:Hackers,Bug # 16519 [1] is another report of confusion regarding trying to use parameters in improper locations - specifically the SET ROLE command within pl/pgsql. I'm re-attaching the doc patch and am adding it to the commitfest.I checked this patch, and I think so it is correct - my comments are just about enhancing by some examplesThank you for the review.v2 attached.I added examples in the two places you noted.Upon re-reading, I decided that opening up the section by including everything then fitting in parameters with an exception for utility commands (without previously/otherwise identifying them) forced some undesirable verbosity. Instead, I opened up with the utility commands as the main body of non-result returning commands and then moved onto delete/insert/update non-returning cases when the subsequent paragraph regarding parameters can then refer to the second class (by way of excluding the first class). This seems to flow better, IMO.I have no objections, but maybe these pages are a little bit unclear generally, because the core of the problem is not described.
Personally I miss a description of the reason why variables cannot be used - the description "variables cannot be used in statements without result" is true, but it is not core information.
In the section "executing commands that don't return results" it does seem like core information...but I get your point.
The important fact is usage of an execution plan or not.
This is already mentioned in the linked-to section:
"Variable substitution currently works only in SELECT, INSERT, UPDATE, and DELETE commands, because the main SQL engine allows query parameters only in these commands. To use a non-constant name or value in other statement types (generically called utility statements), you must construct the utility statement as a string and EXECUTE it."
I didn't feel the need to repeat that material in full in the "no results" section. I left that pointing out the "results" dynamic there would be useful since the original wording seemed to forget about the presence of utility commands altogether which was confusing for that section.
David J.
Re: [patch] [doc] Minor variable related cleanup and rewording of plpgsql docs
From
Pavel Stehule
Date:
po 30. 11. 2020 v 16:06 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:
On Mon, Nov 30, 2020 at 12:51 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:po 30. 11. 2020 v 4:24 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:On Thu, Nov 26, 2020 at 12:49 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:čt 26. 11. 2020 v 6:41 odesílatel David G. Johnston <david.g.johnston@gmail.com> napsal:Hackers,Bug # 16519 [1] is another report of confusion regarding trying to use parameters in improper locations - specifically the SET ROLE command within pl/pgsql. I'm re-attaching the doc patch and am adding it to the commitfest.I checked this patch, and I think so it is correct - my comments are just about enhancing by some examplesThank you for the review.v2 attached.I added examples in the two places you noted.Upon re-reading, I decided that opening up the section by including everything then fitting in parameters with an exception for utility commands (without previously/otherwise identifying them) forced some undesirable verbosity. Instead, I opened up with the utility commands as the main body of non-result returning commands and then moved onto delete/insert/update non-returning cases when the subsequent paragraph regarding parameters can then refer to the second class (by way of excluding the first class). This seems to flow better, IMO.I have no objections, but maybe these pages are a little bit unclear generally, because the core of the problem is not described.Personally I miss a description of the reason why variables cannot be used - the description "variables cannot be used in statements without result" is true, but it is not core information.In the section "executing commands that don't return results" it does seem like core information...but I get your point.The important fact is usage of an execution plan or not.This is already mentioned in the linked-to section:"Variable substitution currently works only in SELECT, INSERT, UPDATE, and DELETE commands, because the main SQL engine allows query parameters only in these commands. To use a non-constant name or value in other statement types (generically called utility statements), you must construct the utility statement as a string and EXECUTE it."I didn't feel the need to repeat that material in full in the "no results" section. I left that pointing out the "results" dynamic there would be useful since the original wording seemed to forget about the presence of utility commands altogether which was confusing for that section.
ok
Pavel
David J.
I wrote:
> I took a stab at doing that, just to see what it might look like.
> I thought it comes out pretty well, really -- see what you think.
Hearing nothing further, I pushed that after another round of
copy-editing. There's still plenty of time to revise it if
anybody has further comments.
regards, tom lane