ne 26. 3. 2023 v 13:32 odesílatel Julien Rouhaud <rjuju...@gmail.com> napsal:
> Hi, > > I just have a few minor wording improvements for the various comments / > documentation you quoted. > > On Sun, Mar 26, 2023 at 08:53:49AM +0200, Pavel Stehule wrote: > > út 21. 3. 2023 v 17:18 odesílatel Peter Eisentraut < > > peter.eisentr...@enterprisedb.com> napsal: > > > > > - What is the purpose of struct Variable? It seems very similar to > > > FormData_pg_variable. At least a comment would be useful. > > > > > > > I wrote comment there: > > > > > > /* > > * The Variable struct is based on FormData_pg_variable struct. Against > > * FormData_pg_variable it can hold node of deserialized expression used > > * for calculation of default value. > > */ > > Did you mean "Unlike" rather than "Against"? > fixed > > > > 0002 > > > > > > expr_kind_allows_session_variables() should have some explanation > > > about criteria for determining which expression kinds should allow > > > variables. > > > > > > > I wrote comment there: > > > > /* > > * Returns true, when expression of kind allows using of > > * session variables. > > + * The session's variables can be used everywhere where > > + * can be used external parameters. Session variables > > + * are not allowed in DDL. Session's variables cannot be > > + * used in constraints. > > + * > > + * The identifier can be parsed as an session variable > > + * only in expression's kinds where session's variables > > + * are allowed. This is the primary usage of this function. > > + * > > + * Second usage of this function is for decision if > > + * an error message "column does not exist" or "column > > + * or variable does not exist" should be printed. When > > + * we are in expression, where session variables cannot > > + * be used, we raise the first form or error message. > > */ > > Maybe > > /* > * Returns true if the given expression kind is valid for session variables > * Session variables can be used everywhere where external parameters can > be > * used. Session variables are not allowed in DDL commands or in > constraints. > * > * An identifier can be parsed as a session variable only for expression > kinds > * where session variables are allowed. This is the primary usage of this > * function. > * > * Second usage of this function is to decide whether "column does not > exist" or > * "column or variable does not exist" error message should be printed. > * When we are in an expression where session variables cannot be used, we > raise > * the first form or error message. > */ > changed > > > > session_variables_ambiguity_warning: There needs to be more > > > information about this. The current explanation is basically just, > > > "warn if your query is confusing". Why do I want that? Why would I > > > not want that? What is the alternative? What are some examples? > > > Shouldn't there be a standard behavior without a need to configure > > > anything? > > > > > > > I enhanced this entry: > > > > + <para> > > + The session variables can be shadowed by column references in a > > query. This > > + is an expected feature. The existing queries should not be > broken > > by creating > > + any session variable, because session variables are shadowed > > always if the > > + identifier is ambiguous. The variables should be named without > > possibility > > + to collision with identifiers of other database objects (column > > names or > > + record field names). The warnings enabled by setting > > <varname>session_variables_ambiguity_warning</varname> > > + should help with finding identifier's collisions. > > Maybe > > Session variables can be shadowed by column references in a query, this is > an > expected behavior. Previously working queries shouldn't error out by > creating > any session variable, so session variables are always shadowed if an > identifier > is ambiguous. Variables should be referenced using an unambiguous > identifier > without any possibility for a collision with identifier of other database > objects (column names or record fields names). The warning messages > emitted > when enabling <varname>session_variables_ambiguity_warning</varname> can > help > finding such identifier collision. > > > + </para> > > + <para> > > + This feature can significantly increase size of logs, and then > it > > is > > + disabled by default, but for testing or development > environments it > > + should be enabled. > > Maybe > > This feature can significantly increase log size, so it's disabled by > default. > For testing or development environments it's recommended to enable it if > you > use session variables. > replaced Thank you very much for these language correctures Regards Pavel p.s. I'll send updated patch after today or tomorrow - I have to fix broken dependency check after rebase
