Re: plpgsql.print_strict_params - Mailing list pgsql-hackers

From Marko Tiikkaja
Subject Re: plpgsql.print_strict_params
Date
Msg-id 5238B84D.30703@joh.to
Whole thread Raw
In response to Re: plpgsql.print_strict_params  (Marko Tiikkaja <marko@joh.to>)
Responses Re: plpgsql.print_strict_params
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: patch: add MAP_HUGETLB to mmap() where supported (WIP)
Next
From: Robert Haas
Date:
Subject: Re: [RFC] Extend namespace of valid guc names