Hi,
Attached is a patch with the following changes:
On 16/09/2013 10:57, I wrote:
> On 9/16/13 8:04 AM, Ian Lawrence Barwick wrote:
>> However the sample function provided in the documentation throws a
>> runtime error due to a missing FROM-clause entry.
>
> Ugh. I'll look into fixing that.
Fixed.
>> This lineis in "exec_get_query_params()" and "exec_get_dynquery_params()":
>>
>> return "(no parameters)";
>>
>> Presumably the message will escape translation and this line should be better
>> written as:
>> return _("(no parameters)");
>
> Nice catch. Will look into this. Another option would be to simply
> omit the DETAIL line if there were no parameters. At this very moment
> I'm thinking that might be a bit nicer.
Decided to remove the DETAIL line in these cases.
>> Also, if the offending query parameter contains a single quote, the output
>> is not escaped:
>>
>> postgres=# select get_userid(E'foo''');
>> ERROR: query returned more than one row
>> DETAIL: p1 = 'foo''
>> CONTEXT: PL/pgSQL function get_userid(text) line 9 at SQL statement
>>
>> Not sure if that's a real problem though.
>
> Hmm.. I should probably look at what happens when the parameters to a
> prepared statement are currently logged and imitate that.
Yup, they're escaped. Did just that. Also copied the "parameters: "
prefix on the DETAIL line from there.
>> The functions added in pl_exec.c - "exec_get_query_params()" and
>> "exec_get_dynquery_params()" do strike me as potentially misnamed,
>> as they don't actually execute anything but are more utility
>> functions for generating formatted output.
>>
>> Maybe "format_query_params()" etc. would be better?
>
> Agreed, they could use some renaming.
I went with format_expr_params and format_preparedparamsdata.
>> * Are the comments sufficient and accurate?
>> "exec_get_query_params()" and "exec_get_dynquery_params()"
>> could do with some brief comments explaining their purpose.
>
> Agreed.
Still agreeing with both of us, added a comment each.
I also added the new keywords to the unreserved_keywords production, as
per the instructions near the beginning of pl_gram.y.
Regards,
Marko Tiikkaja