Horton
Menu

Thread

  1. BUG #4027: backslash escaping not disabled in plpgsql

    Jonathan Guthrie <jguthrie@brokersys.com> — 2008-03-11T21:26:56Z

    The following bug has been logged online:

Bug reference:      4027
Logged by:          Jonathan Guthrie
Email address:      jguthrie@brokersys.com
PostgreSQL version: 8.3.0
Operating system:   Debian Gnu/Linux "unstable" 2.6.24
Description:        backslash escaping not disabled in plpgsql
Details: 

I have set the standard_conforming_strings to "on" in my settings, and have
verified it by executing a

select '\';

which works fine and produces the expected:
  ?column? 
----------
 \
(1 row)

However, when I attempt to define this function:

create function foo (out r refcursor) as $bar$
begin
 open r for
   select * from user_data
   where name_first like name escape '\';
end; $bar$ language plpgsql;

it complains about an unterminated string. ("ERROR:  unterminated string") 
However, if I double the backslashes, it compiles just fine, and does not
emit a warning even though escape_string_warning is also set to 'on'.  As
expected, the system does emit a warning when I double the backslashes and
when standard_conforming_strings is set to 'off'.  I also have
backslash_quote set to 'off', but it doesn't seem to change anything in this
case.

I believe that this is incorrect behavior and that the backslash should be
just a character in that string when standard_conforming_strings is set to
'on'.

  2. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2008-03-12T01:01:32Z

    "Jonathan Guthrie" <jguthrie@brokersys.com> writes:
> I have set the standard_conforming_strings to "on" in my settings ...
> However, when I attempt to define this function:

> create function foo (out r refcursor) as $bar$
> begin
>  open r for
>    select * from user_data
>    where name_first like name escape '\';
> end; $bar$ language plpgsql;

plpgsql does not consider standard_conforming_strings --- it still uses
backslash escaping in its function bodies regardless.  Since the
language itself is not standardized, I see no particular reason that
standard_conforming_strings should govern it.  I believe the reason for
not changing it was that it seemed too likely to break existing
functions, with potentially nasty consequences if they chanced to be
security definers.

			regards, tom lane

  3. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Peter Eisentraut <peter_e@gmx.net> — 2008-03-12T06:54:09Z

    Tom Lane wrote:
> plpgsql does not consider standard_conforming_strings --- it still uses
> backslash escaping in its function bodies regardless.  Since the
> language itself is not standardized, I see no particular reason that
> standard_conforming_strings should govern it.

I think plpgsql should behave either consistently with the rest of PostgreSQL 
or with Oracle, which it is copied from.

> I believe the reason for 
> not changing it was that it seemed too likely to break existing
> functions, with potentially nasty consequences if they chanced to be
> security definers.

Is this actually true or did we just forget it? :-)

  4. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2008-03-12T06:59:21Z

    Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> I believe the reason for 
>> not changing it was that it seemed too likely to break existing
>> functions, with potentially nasty consequences if they chanced to be
>> security definers.

> Is this actually true or did we just forget it? :-)

I recall thinking about the point.  The decision could've been wrong ...

			regards, tom lane

  5. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Bruce Momjian <bruce@momjian.us> — 2008-03-12T13:18:41Z

    Peter Eisentraut wrote:
> Tom Lane wrote:
> > plpgsql does not consider standard_conforming_strings --- it still uses
> > backslash escaping in its function bodies regardless.  Since the
> > language itself is not standardized, I see no particular reason that
> > standard_conforming_strings should govern it.
> 
> I think plpgsql should behave either consistently with the rest of PostgreSQL 
> or with Oracle, which it is copied from.

Agreed. standard_conforming_strings should affect _all_ strings.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://postgres.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  6. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Peter Eisentraut <peter_e@gmx.net> — 2008-03-12T19:28:59Z

    Bruce Momjian wrote:
> Agreed. standard_conforming_strings should affect _all_ strings.

We might need another transition period over a few releases with a 
separate "plpgsql_standard_conforming_strings" parameter.  Just changing it 
immediately is perhaps a bit risky.

  7. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Bruce Momjian <bruce@momjian.us> — 2008-03-12T19:37:00Z

    Peter Eisentraut wrote:
> Bruce Momjian wrote:
> > Agreed. standard_conforming_strings should affect _all_ strings.
> 
> We might need another transition period over a few releases with a 
> separate "plpgsql_standard_conforming_strings" parameter.  Just changing it 
> immediately is perhaps a bit risky.

We haven't even enabled standard_conforming_strings by default yet.  It
was added as changeable in 8.2.  Is this never going to be enabled by
default?

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://postgres.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  8. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Bruce Momjian <bruce@momjian.us> — 2008-06-23T19:50:16Z

    Peter Eisentraut wrote:
> Tom Lane wrote:
> > plpgsql does not consider standard_conforming_strings --- it still uses
> > backslash escaping in its function bodies regardless.  Since the
> > language itself is not standardized, I see no particular reason that
> > standard_conforming_strings should govern it.
> 
> I think plpgsql should behave either consistently with the rest of PostgreSQL 
> or with Oracle, which it is copied from.
> 
> > I believe the reason for 
> > not changing it was that it seemed too likely to break existing
> > functions, with potentially nasty consequences if they chanced to be
> > security definers.
> 
> Is this actually true or did we just forget it? :-)

I would like to add a TODO item for this, but I am concerned that people
running functions with different standard_conforming_strings values
would have function syntax errors on mismatch.  Is that acceptable?

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  9. Re: BUG #4027: backslash escaping not disabled in plpgsql

    Bruce Momjian <bruce@momjian.us> — 2008-12-16T03:34:41Z

    Peter Eisentraut wrote:
> Tom Lane wrote:
> > plpgsql does not consider standard_conforming_strings --- it still uses
> > backslash escaping in its function bodies regardless.  Since the
> > language itself is not standardized, I see no particular reason that
> > standard_conforming_strings should govern it.
> 
> I think plpgsql should behave either consistently with the rest of PostgreSQL 
> or with Oracle, which it is copied from.
> 
> > I believe the reason for 
> > not changing it was that it seemed too likely to break existing
> > functions, with potentially nasty consequences if they chanced to be
> > security definers.
> 
> Is this actually true or did we just forget it? :-)

Did we ever address this?

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  10. Re: [BUGS] BUG #4027: backslash escaping not disabled in plpgsql

    Bruce Momjian <bruce@momjian.us> — 2009-04-09T15:16:45Z

    Peter Eisentraut wrote:
> Tom Lane wrote:
> > plpgsql does not consider standard_conforming_strings --- it still uses
> > backslash escaping in its function bodies regardless.  Since the
> > language itself is not standardized, I see no particular reason that
> > standard_conforming_strings should govern it.
> 
> I think plpgsql should behave either consistently with the rest of PostgreSQL 
> or with Oracle, which it is copied from.
> 
> > I believe the reason for 
> > not changing it was that it seemed too likely to break existing
> > functions, with potentially nasty consequences if they chanced to be
> > security definers.
> 
> Is this actually true or did we just forget it? :-)

I have added this TODO item:

	Consider honoring standard_conforming_strings in PL/pgSQL function
	bodies
	
	    * http://archives.postgresql.org/pgsql-bugs/2008-03/msg00102.php 

Are we every going to enable standard_conforming_strings by default?  If
not, I will remove the TODO item mentiong this. 
standard_conforming_strings was added in Postgres 8.1, and
escape_string_warning was enabled in 8.2.

I think the big issue is that having standard_conforming_strings affect
function behavior introduces the same problems we have had in the past
of having a GUC affect function behavior.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  11. Re: Re: [BUGS] BUG #4027: backslash escaping not disabled in plpgsql

    Robert Haas <robertmhaas@gmail.com> — 2009-04-09T15:21:54Z

    On Thu, Apr 9, 2009 at 11:16 AM, Bruce Momjian <bruce@momjian.us> wrote:
> Peter Eisentraut wrote:
>> Tom Lane wrote:
>> > plpgsql does not consider standard_conforming_strings --- it still uses
>> > backslash escaping in its function bodies regardless.  Since the
>> > language itself is not standardized, I see no particular reason that
>> > standard_conforming_strings should govern it.
>>
>> I think plpgsql should behave either consistently with the rest of PostgreSQL
>> or with Oracle, which it is copied from.
>>
>> > I believe the reason for
>> > not changing it was that it seemed too likely to break existing
>> > functions, with potentially nasty consequences if they chanced to be
>> > security definers.
>>
>> Is this actually true or did we just forget it? :-)
>
> I have added this TODO item:
>
>        Consider honoring standard_conforming_strings in PL/pgSQL function
>        bodies
>
>            * http://archives.postgresql.org/pgsql-bugs/2008-03/msg00102.php
>
> Are we every going to enable standard_conforming_strings by default?  If
> not, I will remove the TODO item mentiong this.
> standard_conforming_strings was added in Postgres 8.1, and
> escape_string_warning was enabled in 8.2.
>
> I think the big issue is that having standard_conforming_strings affect
> function behavior introduces the same problems we have had in the past
> of having a GUC affect function behavior.

I think this should wait at least one more release.  Based on my
experience, there are probably a LOT of applications out there that
have yet to be updated.

It wouldn't bother me if we never enabled it by default, either.  I'm
just -1 on doing it now.

...Robert

  12. Re: [BUGS] BUG #4027: backslash escaping not disabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-09T15:36:23Z

    Bruce Momjian <bruce@momjian.us> wrote:
> standard_conforming_strings was added in Postgres 8.1, and
> escape_string_warning was enabled in 8.2.
 
Other way around -- the warning was available in 8.1; the standard
character string literals were available in 8.2.

> I think the big issue is that having standard_conforming_strings
> affect function behavior introduces the same problems we have had in
> the past of having a GUC affect function behavior.
 
Can't that be managed with this CREATE FUNCTION option?:
 
SET configuration_parameter { TO value | = value | FROM CURRENT }
 
I would like to see standard character string literals at least
available in PL/pgSQL, although I don't personally care whether it is
the default or whether I need to specify it with the above option. 
Might it not confuse people to have this GUC behave differently than
others, though?
 
-Kevin

  13. Re: Re: [BUGS] BUG #4027: backslash escaping not disabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-09T16:06:16Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Bruce Momjian <bruce@momjian.us> wrote:
>> I think the big issue is that having standard_conforming_strings
>> affect function behavior introduces the same problems we have had in
>> the past of having a GUC affect function behavior.
 
> Can't that be managed with this CREATE FUNCTION option?:
> SET configuration_parameter { TO value | = value | FROM CURRENT }

It can be, the question is whether we're prepared to break everything
under the sun until people add that.

			regards, tom lane

  14. Re: Re: [BUGS] BUG #4027: backslash escaping not disabled inplpgsql

    Bruce Momjian <bruce@momjian.us> — 2009-04-09T16:18:32Z

    Tom Lane wrote:
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> > Bruce Momjian <bruce@momjian.us> wrote:
> >> I think the big issue is that having standard_conforming_strings
> >> affect function behavior introduces the same problems we have had in
> >> the past of having a GUC affect function behavior.
>  
> > Can't that be managed with this CREATE FUNCTION option?:
> > SET configuration_parameter { TO value | = value | FROM CURRENT }
> 
> It can be, the question is whether we're prepared to break everything
> under the sun until people add that.

I think we would first have to agree to issue escape_string_warning
warnings for code in PL/pgSQL functions, then think about having
standard_conforming_strings control PL/pgSQL behavior;  this is what we
did with SQL and it seems to have worked.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  15. Re: Re: [BUGS] BUG #4027: backslash escaping not disabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-09T16:26:52Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote: 
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
>> Can't that be managed with this CREATE FUNCTION option?:
>> SET configuration_parameter { TO value | = value | FROM CURRENT }
> 
> It can be, the question is whether we're prepared to break
everything
> under the sun until people add that.
 
Well, surely the 8.3 behavior is not what we want.
 
(1)  The plpgsql parser identifies the boundaries of the string based
on backslash escapes.
 
(2)  The character string literal is interpreted based on the GUC
setting the first time the function is executed (and presumably the
first time executed after the function's invalidated).
 
(3)  Subsequent changes to the GUC don't affect how it's interpreted.
 
scca=# show standard_conforming_strings ;
 standard_conforming_strings
-----------------------------
 on
(1 row)

scca=# create or replace function kjgtest() returns text language
plpgsql immutable strict as $$ begin return '\x49'; end; $$;
CREATE FUNCTION
scca=# select * from kjgtest();
 kjgtest
---------
 \x49
(1 row)

scca=# set standard_conforming_strings = off;
SET
scca=# select * from kjgtest();
 kjgtest
---------
 \x49
(1 row)

scca=# create or replace function kjgtest() returns text language
plpgsql immutable strict as $$ begin return '\x49'; end; $$;
CREATE FUNCTION
scca=# select * from kjgtest();
 kjgtest
---------
 I
(1 row)

scca=# set standard_conforming_strings = on;
SET
scca=# select * from kjgtest();
 kjgtest
---------
 I
(1 row)

scca=# create or replace function kjgtest() returns text language
plpgsql immutable strict as $$ begin return '\x49'; end; $$;
CREATE FUNCTION
scca=# set standard_conforming_strings = off;
SET
scca=# select * from kjgtest();
 kjgtest
---------
 I
(1 row)
 
-Kevin

  16. Re: Re: [BUGS] BUG #4027: backslash escaping not disabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-09T16:55:16Z

    Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> It can be, the question is whether we're prepared to break everything
>> under the sun until people add that.

> I think we would first have to agree to issue escape_string_warning
> warnings for code in PL/pgSQL functions, then think about having
> standard_conforming_strings control PL/pgSQL behavior;  this is what we
> did with SQL and it seems to have worked.

Well, considering that we are still afraid to pull the trigger on
changing the standard_conforming_strings default, it's a bit premature
to claim that it "worked" for SQL.  But I agree that some kind of
stepwise process will be necessary if we want to try to change this.

IIRC there was some discussion of using plpgsql's (undocumented) #option
syntax to control this, rather than having a GUC that would be specific
to plpgsql.

			regards, tom lane

  17. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-09T18:30:21Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote: 
> Well, surely the 8.3 behavior is not what we want.
 
Unless I'm missing something, plpgsql *already* effectively recognizes
and respects the standard_conforming_strings GUC *except* as the last
character of a conforming string literal within the procedure body,
and then not always. Am I missing something here?
 
scca=# set standard_conforming_strings = on;
SET
scca=# create or replace function kjgtest() returns text language
plpgsql immutable as 'begin return \'\x49\'; end;';
Expanded display is on.
Invalid command \';. Try \? for help.
scca=# \x
Expanded display is off.
scca-# create or replace function kjgtest() returns text language
plpgsql immutable as $$ begin return '\x49\'; end; $$;
ERROR:  syntax error at or near "create"
LINE 2: create or replace function kjgtest() returns text language
p...
        ^
scca=# create or replace function kjgtest() returns text language
plpgsql immutable as $$ begin return '\x49\\'; end; $$;
CREATE FUNCTION
scca=# select kjgtest();
 kjgtest
---------
 \x49\\
(1 row)

scca=# set standard_conforming_strings = off;
SET
scca=# create or replace function kjgtest() returns text language
plpgsql immutable as 'begin return \'\x49\'; end;';
CREATE FUNCTION
scca=# select kjgtest();
 kjgtest
---------
 I
(1 row)

scca=# create or replace function kjgtest() returns text language
plpgsql immutable as $$ begin return '\x49\'; end; $$;
ERROR:  unterminated string
CONTEXT:  compile of PL/pgSQL function "kjgtest" near line 1
scca=# create or replace function kjgtest() returns text language
plpgsql immutable as $$ begin return '\x49\\'; end; $$;
CREATE FUNCTION
scca=# select kjgtest();
 kjgtest
---------
 I\
(1 row)
 
Given this behavior, how much could be working for
standard_conforming_strings = on which would break with more complete
support?  Maybe this particular GUC should default to an implied SET
standard_conforming_strings FROM CURRENT and the plpgsql parser should
use it?  Can anyone show a working case that would break with that?
 
-Kevin

  18. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-09T18:56:46Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Unless I'm missing something, plpgsql *already* effectively recognizes
> and respects the standard_conforming_strings GUC *except* as the last
> character of a conforming string literal within the procedure body,
> and then not always. Am I missing something here?

Yes --- I think you are confusing parsing of the string literal that
is the argument of CREATE FUNCTION with the parsing that the plpgsql
interpreter does on the function body once it gets it.  In particular,
this example:

create or replace function kjgtest() returns text language
plpgsql immutable as $$ begin return 'foo\'; end; $$;

fails regardless of the standard_conforming_strings setting, because
the plpgsql interpreter considers the backslash to escape the quote
regardless.

			regards, tom lane

  19. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-09T19:11:15Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote: 
> I think you are confusing parsing of the string literal that
> is the argument of CREATE FUNCTION with the parsing that the plpgsql
> interpreter does on the function body once it gets it.  In
> particular, this example:
> 
> create or replace function kjgtest() returns text language
> plpgsql immutable as $$ begin return 'foo\'; end; $$;
> 
> fails regardless of the standard_conforming_strings setting, because
> the plpgsql interpreter considers the backslash to escape the quote
> regardless.
 
Oh, I'm not confused about that at all.  I'm arguing that it's a bad
idea.  I agree with the OP that this is a bug.  Did you look at my
other examples of behavior?  In particular:
 
scca=# create or replace function kjgtest() returns text language
plpgsql immutable as $$ begin return '\x49\\'; end; $$;
CREATE FUNCTION
scca=# select kjgtest();
 kjgtest
---------
 \x49\\
(1 row)
 
Can you show one case where having plgpsql parse the function body
based on the standard_conforming_strings GUC would break *anything*
that now works?  That's an allegation which I haven't been able to
confirm, so I'm wondering about the basis.
 
-Kevin

  20. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-09T21:10:44Z

    I wrote: 
> Tom Lane <tgl@sss.pgh.pa.us> wrote: 
>> I think you are confusing parsing of the string literal that
>> is the argument of CREATE FUNCTION with the parsing that the
plpgsql
>> interpreter does on the function body once it gets it.
 
> Oh, I'm not confused about that at all.  I'm arguing that it's a bad
> idea.
 
To be more explicit, I see that there is a third parser phase -- when
the function is planned, the original contents of the character string
literal are passed to the normal PostgreSQL execution engine, which
parses them again, potentially using different rules from those used
by the plpgsql interpreter.  I maintain that having the execution
engine use different rules for looking at the value of the literal
than the plpgsql parser used to find the boundaries of the literal
is where the weird corner case bugs come in.
 
For someone using string literal '\x49\\' in a plpgsql function, the
plpgsql parser sees it as a two character string, but when the
function is actually run, depending on whether the first execution is
using standard string literals, this can be either a two character or
a six character string.  Unless the coder of the function uses the SET
option in declaring the function, they don't know what value will be
used at run time, and it may change from run to run.
 
It seems to me that we already have exactly the kinds of problems you
say you want to avoid, and that there is an obvious fix to avoid them.
 
-Kevin

  21. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T18:21:23Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote: 
>> I think you are confusing parsing of the string literal that
>> is the argument of CREATE FUNCTION with the parsing that the plpgsql
>> interpreter does on the function body once it gets it.
 
> Oh, I'm not confused about that at all.  I'm arguing that it's a bad
> idea.  I agree with the OP that this is a bug.  Did you look at my
> other examples of behavior?

I ignored all the ones that used non-dollar-quote syntax for the overall
function body, since they are just confusing the issue.

> Can you show one case where having plgpsql parse the function body
> based on the standard_conforming_strings GUC would break *anything*
> that now works?

regression=# create function foo() returns int as $$
regression$# begin
regression$#   raise notice 'foo\'s bar';
regression$#   return 1;
regression$# end$$ language plpgsql;
CREATE FUNCTION
regression=# select foo();
NOTICE:  foo's bar
 foo 
-----
   1
(1 row)

In this case the string literal isn't actually ever passed to the main
SQL engine, so the SQL quoting rules aren't relevant.  (I don't remember
offhand if anything besides RAISE works that way.)

It may be that this isn't a very important case, but to claim that
it doesn't exist is simply wrong.

			regards, tom lane

  22. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-10T18:40:06Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote: 
>> Can you show one case where having plgpsql parse the function body
>> based on the standard_conforming_strings GUC would break *anything*
>> that now works?
> 
> regression=# create function foo() returns int as $$
> regression$# begin
> regression$#   raise notice 'foo\'s bar';
> regression$#   return 1;
> regression$# end$$ language plpgsql;
> CREATE FUNCTION
> regression=# select foo();
> NOTICE:  foo's bar
>  foo 
> -----
>    1
> (1 row)
> 
> In this case the string literal isn't actually ever passed to the
> main SQL engine, so the SQL quoting rules aren't relevant.  (I don't
> remember offhand if anything besides RAISE works that way.)
> 
> It may be that this isn't a very important case, but to claim that
> it doesn't exist is simply wrong.
 
OK, I didn't try that.  Point taken.  It is a bigger mess than I
thought then.
 
The aspect of 8.3 behavior that concerns me most is that neither the
author of a function, nor anyone using it, can control or predict
which way a string literal with a backslash will be interpreted,
unless the author explicitly specifies the SET
standard_conforming_strings clause in the function declaration.  I'm
betting that most people writing and using plpgsql functions don't
know that.  Any thoughts about what can or should be done about that?
 
-Kevin

  23. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T18:57:10Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> The aspect of 8.3 behavior that concerns me most is that neither the
> author of a function, nor anyone using it, can control or predict
> which way a string literal with a backslash will be interpreted,
> unless the author explicitly specifies the SET
> standard_conforming_strings clause in the function declaration.

Yeah.  This is one reason why I'm still afraid to flip the default
value of standard_conforming_strings --- there seems too much risk
of widespread breakage.

I don't have a good solution for it, but I agree it's a problem.

			regards, tom lane

  24. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Brendan Jurd <direvus@gmail.com> — 2009-04-10T19:11:22Z

    On Sat, Apr 11, 2009 at 4:40 AM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
> The aspect of 8.3 behavior that concerns me most is that neither the
> author of a function, nor anyone using it, can control or predict
> which way a string literal with a backslash will be interpreted,
> unless the author explicitly specifies the SET
> standard_conforming_strings clause in the function declaration.  I'm
> betting that most people writing and using plpgsql functions don't
> know that.  Any thoughts about what can or should be done about that?

Isn't this exactly the same problem that application authors have been
facing with SQL in their code?

Namely, if there's a backslash anywhere in a string literal you
*cannot* leave it as a bare single-quoted string literal.  You need to
decide whether you want the backslash treated as an escape character
(and therefore use E quoting), or as a backslash (and therefore use $$
quoting).

Until you've done that for every single string literal with a
backslash, your application isn't ready for
standard_conforming_strings to be switched on.

I agree that there are probably a great many app authors out there who
don't realise how very boned they might be if the default GUC gets
changed and they haven't prepared their SQL to cope.

Cheers,
BJ

  25. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Bruce Momjian <bruce@momjian.us> — 2009-04-10T19:13:10Z

    Brendan Jurd wrote:
> On Sat, Apr 11, 2009 at 4:40 AM, Kevin Grittner
> <Kevin.Grittner@wicourts.gov> wrote:
> > The aspect of 8.3 behavior that concerns me most is that neither the
> > author of a function, nor anyone using it, can control or predict
> > which way a string literal with a backslash will be interpreted,
> > unless the author explicitly specifies the SET
> > standard_conforming_strings clause in the function declaration. ?I'm
> > betting that most people writing and using plpgsql functions don't
> > know that. ?Any thoughts about what can or should be done about that?
> 
> Isn't this exactly the same problem that application authors have been
> facing with SQL in their code?
> 
> Namely, if there's a backslash anywhere in a string literal you
> *cannot* leave it as a bare single-quoted string literal.  You need to
> decide whether you want the backslash treated as an escape character
> (and therefore use E quoting), or as a backslash (and therefore use $$
> quoting).
> 
> Until you've done that for every single string literal with a
> backslash, your application isn't ready for
> standard_conforming_strings to be switched on.
> 
> I agree that there are probably a great many app authors out there who
> don't realise how very boned they might be if the default GUC gets
> changed and they haven't prepared their SQL to cope.

I assume those authors are getting warnings, which is something we don't
for PL/pgSQL now.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  26. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-10T19:15:48Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote: 
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
>> The aspect of 8.3 behavior that concerns me most is that neither
>> the author of a function, nor anyone using it, can control or
>> predict which way a string literal with a backslash will be
>> interpreted, unless the author explicitly specifies the SET
>> standard_conforming_strings clause in the function declaration.
> 
> Yeah.  This is one reason why I'm still afraid to flip the default
> value of standard_conforming_strings --- there seems too much risk
> of widespread breakage.
> 
> I don't have a good solution for it, but I agree it's a problem.
 
Now that I see that string literals are currently interpreted
inconsistently, I don't think there's any way to get to a sane
behavior without risking some breakage somewhere.  If, as I've seen
some people assert, most people aren't setting the
standard_conforming_strings = on, it would seem to be reasonable to
put the risk with that 'on' setting.
 
Let me ask this -- If we were to change the plpgsql parser to pay
attention to the GUC, it couldn't break anything for any environment
which always has the GUC 'off', could it?
 
If not, I am having a hard time seeing a smoother transition than to
change the plpgsql parser to use the GUC, and to have the CREATE
FUNCTION statement make a special case of defaulting this GUC to FROM
CURRENT.  Making an exception of this offends a little, but not as
badly as unpredictable runtime behavior.
 
An advantage of this approach is that it would be just another place
to check your string literals when and if you go to switch over to
standard literals.
 
Whether to ever change the default behavior over to the standard is
more of a "marketing" decision than a technical one, I think.
 
-Kevin

  27. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T19:24:16Z

    Bruce Momjian <bruce@momjian.us> writes:
> Brendan Jurd wrote:
>> I agree that there are probably a great many app authors out there who
>> don't realise how very boned they might be if the default GUC gets
>> changed and they haven't prepared their SQL to cope.

> I assume those authors are getting warnings, which is something we don't
> for PL/pgSQL now.

To the extent that the strings are getting passed through to the main
SQL engine, they do get warnings now, and pretty noisy ones:

regression=# create function foo2() returns text as $$
begin
  return 'foo\'s bar';
end$$ language plpgsql;
WARNING:  nonstandard use of \' in a string literal
LINE 1: SELECT  'foo\'s bar'
                ^
HINT:  Use '' to write quotes in strings, or use the escape string syntax (E'...').
QUERY:  SELECT  'foo\'s bar'
CONTEXT:  SQL statement in PL/PgSQL function "foo2" near line 2
CREATE FUNCTION
regression=# select foo2();
WARNING:  nonstandard use of \' in a string literal
LINE 1: SELECT  'foo\'s bar'
                ^
HINT:  Use '' to write quotes in strings, or use the escape string syntax (E'...').
QUERY:  SELECT  'foo\'s bar'
CONTEXT:  PL/pgSQL function "foo2" line 2 at RETURN
   foo2    
-----------
 foo's bar
(1 row)

It's the corner cases where plpgsql doesn't pass strings through that
are worrisome.  It's possible that RAISE is the only such case ---
anyone want to check?

Actually, what this thread is leading me towards is the idea that almost
nobody really has standard_conforming_strings turned on in production
(except maybe with apps ported from Oracle or someplace else).  If they
did, we'd be seeing more complaints about plpgsql not working properly.
So maybe we *could* change plpgsql to honor the GUC without anyone
noticing too much.

			regards, tom lane

  28. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T19:29:39Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Let me ask this -- If we were to change the plpgsql parser to pay
> attention to the GUC, it couldn't break anything for any environment
> which always has the GUC 'off', could it?

Right, because the behavior wouldn't actually change.

I'm starting to lean in the same direction --- the current plpgsql
behavior with the GUC 'on' is sufficiently broken that it seems unlikely
anyone is doing much with plpgsql and that setting.

It still remains that actually flipping the default would probably
provoke lots of breakage, but plpgsql's current behavior doesn't
help that.

			regards, tom lane

  29. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Bruce Momjian <bruce@momjian.us> — 2009-04-10T19:42:37Z

    Tom Lane wrote:
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> > Let me ask this -- If we were to change the plpgsql parser to pay
> > attention to the GUC, it couldn't break anything for any environment
> > which always has the GUC 'off', could it?
> 
> Right, because the behavior wouldn't actually change.
> 
> I'm starting to lean in the same direction --- the current plpgsql
> behavior with the GUC 'on' is sufficiently broken that it seems unlikely
> anyone is doing much with plpgsql and that setting.
> 
> It still remains that actually flipping the default would probably
> provoke lots of breakage, but plpgsql's current behavior doesn't
> help that.

It would be nice to know if we are ever going to set
standard_conforming_strings to on.  If not, we can remove the TODO item.
The bigger question is if we aren't going to turn it on was there any
value to setting escape_string_warning to on in 8.2?  We required a lot
of users to prefix their strings with 'E'.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  30. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-10T19:54:12Z

    Bruce Momjian <bruce@momjian.us> wrote:
> It would be nice to know if we are ever going to set
> standard_conforming_strings to on.
 
My personal bias is to go to the standard behavior as the default at
some point.  For legacy reasons, I don't know that you would ever want
to remove the setting; especially since I don't think it adds much
code if you're going to support the E'...' literals.  The ugliest
thing about this GUC is that it adds some complications to the flex
code, but it doesn't seem that bad to me.
 
-Kevin

  31. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Bruce Momjian <bruce@momjian.us> — 2009-04-10T19:56:02Z

    Kevin Grittner wrote:
> Bruce Momjian <bruce@momjian.us> wrote:
> > It would be nice to know if we are ever going to set
> > standard_conforming_strings to on.
>  
> My personal bias is to go to the standard behavior as the default at
> some point.  For legacy reasons, I don't know that you would ever want
> to remove the setting; especially since I don't think it adds much
> code if you're going to support the E'...' literals.  The ugliest
> thing about this GUC is that it adds some complications to the flex
> code, but it doesn't seem that bad to me.

Agreed, we would probably never remove standard_conforming_strings.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

  32. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T20:07:17Z

    Bruce Momjian <bruce@momjian.us> writes:
> Kevin Grittner wrote:
>> My personal bias is to go to the standard behavior as the default at
>> some point.  For legacy reasons, I don't know that you would ever want
>> to remove the setting; especially since I don't think it adds much
>> code if you're going to support the E'...' literals.  The ugliest
>> thing about this GUC is that it adds some complications to the flex
>> code, but it doesn't seem that bad to me.

> Agreed, we would probably never remove standard_conforming_strings.

Yeah, I don't see that happening either.  I agree with Kevin that it
would be nice to flip the default at some point, but I'm afraid it's a
long way off yet.

Back to the point at hand: do we want to look at making plpgsql respect
the GUC?  I think it's a bit trickier than it looks, because we don't
want duplicate warnings from both plpgsql and the main parser for
strings that get fed through.  I'm inclined to deal with the special
case (RAISE and anything else similar) by changing the code so that we
*do* feed the string literal through the main parser, not for any
functional effect but just to have it throw the right warnings/errors.
Otherwise the plpgsql lexer has to somehow know when to warn and when
not, which'd be a mess.

			regards, tom lane

  33. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-10T20:23:52Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote: 
> do we want to look at making plpgsql respect the GUC?
 
+1
 
> I'm inclined to deal with the special case (RAISE and anything else
> similar) by changing the code so that we *do* feed the string
> literal through the main parser, not for any functional effect but
> just to have it throw the right warnings/errors.
 
+1
 
-Kevin

  34. Re: Re: [BUGS] BUG #4027: backslash escaping notdisabled inplpgsql

    Josh Berkus <josh@agliodbs.com> — 2009-04-10T20:35:06Z

    Tom,

> Actually, what this thread is leading me towards is the idea that almost
> nobody really has standard_conforming_strings turned on in production
> (except maybe with apps ported from Oracle or someplace else).  If they
> did, we'd be seeing more complaints about plpgsql not working properly.
> So maybe we *could* change plpgsql to honor the GUC without anyone
> noticing too much.

Actually, a lot of people are using $escapes$ for all nested quotes in 
plpgsql.  So they wouldn't notice the problem.

-- 
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

  35. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Andrew Gierth <andrew@tao11.riddles.org.uk> — 2009-04-10T20:38:27Z

    >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> Back to the point at hand: do we want to look at making plpgsql
 Tom> respect the GUC?

Surely what matters is the value of the GUC at the time that you did
the CREATE FUNCTION, not the value at the time you happen to be
calling it?

-- 
Andrew (irc:RhodiumToad)

  36. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabledinplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-10T20:46:09Z

    Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: 
> Surely what matters is the value of the GUC at the time that you did
> the CREATE FUNCTION, not the value at the time you happen to be
> calling it?
 
Well, that's a change I'm arguing for.  That would require both the
plpgsql parser change Tom is talking about, and a change to CREATE
FUNCTION such that there is an implied SET standard_compliant_strings
FROM CURRENT -- which is something I've suggested a couple times;
there's been no explicit response to that.
 
See back here in the thread for some behavior which surprised me:
 
http://archives.postgresql.org/pgsql-hackers/2009-04/msg00519.php
 
-Kevin

  37. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabled inplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T21:16:14Z

    Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>  Tom> Back to the point at hand: do we want to look at making plpgsql
>  Tom> respect the GUC?

> Surely what matters is the value of the GUC at the time that you did
> the CREATE FUNCTION, not the value at the time you happen to be
> calling it?

No, it isn't, and that's not the immediate problem anyway --- the
immediate problem is that plpgsql doesn't respect *any* value of
the GUC.

			regards, tom lane

  38. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabledinplpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2009-04-10T21:24:17Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Well, that's a change I'm arguing for.  That would require both the
> plpgsql parser change Tom is talking about, and a change to CREATE
> FUNCTION such that there is an implied SET standard_compliant_strings
> FROM CURRENT -- which is something I've suggested a couple times;
> there's been no explicit response to that.

If you want one: it seems like a really bad idea.  Aside from the sheer
ugliness of special-casing one particular GUC, it would break existing
pg_dump files, since pg_dump has no idea that its setting of
standard_conforming_strings might influence the behavior of functions
it defines.

I don't actually see that standard_conforming_strings is worse than
search_path or half a dozen other settings that will influence the
semantics of SQL queries.  If anything it's less bad than those since
it's less likely to break things silently.  The whole topic just
illustrates that "invent a GUC" is not a pain-free solution to handling
definitional conflicts.

			regards, tom lane

  39. Re: Re: [BUGS] BUG #4027: backslash escapingnotdisabledinplpgsql

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-04-13T19:06:33Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote: 
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
>> a change to CREATE FUNCTION such that there is an implied SET
>> standard_compliant_strings FROM CURRENT
 
Hopefully obvious, I meant standard_conforming_strings.
 
> it seems like a really bad idea.
 
Then perhaps a note in the PL/pgSQL docs about the importance of
specifying that clause if the function contains any character string
literals which include a backslash?  Such a note should probably point
out that without this clause, the runtime value of any such literal
will be dependent on the value of standard_conforming_strings when the
plan is generated.
 
I think that many will find that behavior surprising; so if it's not
feasible to change it, we should at least document it.
 
-Kevin