Thread

1. string escaping in tutorial/syscat.source

   Josh Kupershmidt <schmiddy@gmail.com> — 2012-10-15T03:53:51Z

   Hi all,
   It seems the queries in ./src/tutorial/syscat.source use string
   escaping with the assumption that standard_conforming_strings is off,
   and thus give wrong results with modern versions. A simple fix is
   attached.
   Josh
   

2. string escaping in tutorial/syscat.source

   Jeff Janes <jeff.janes@gmail.com> — 2013-01-16T01:35:15Z

   On Sun, Oct 14, 2012 at 8:53 PM, Josh Kupershmidt
   <schmiddy@gmail.com<javascript:;>>
   wrote:
   > Hi all,
   > It seems the queries in ./src/tutorial/syscat.source use string
   > escaping with the assumption that standard_conforming_strings is off,
   > and thus give wrong results with modern versions. A simple fix is
   > attached.
   
   Hi Josh,
   
   Do you propose back-patching this?  You could argue that this is a bug in
   9.1 and 9.2.   Before that, they generate deprecation warnings, but do not
   give the wrong answer.
   
   If it is only to be applied to HEAD, or only to 9.1, 9.2, and HEAD, then
   this part seems to be unnecessary and I think should be removed (setting a
   value to its default is more likely to cause confusion than remove
   confusion):
   
   SET standard_conforming_strings TO on;
   
   and the corresponding reset as well.
   
   Other than that, it does what it says (fix a broken example), does it
   cleanly, we want this, and I have no other concerns.
   
   I guess the src/tutorial directory could participate in regression tests,
   in which case this problem would have been detected when introduced, but I
   don't think I can demand that you invent regression tests for a feature you
   are just fixing rather than creating.
   
   Thanks for the patch,
   
   Jeff
   

3. Re: string escaping in tutorial/syscat.source

   Josh Kupershmidt <schmiddy@gmail.com> — 2013-01-16T04:54:56Z

   On Tue, Jan 15, 2013 at 6:35 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
   
   > Do you propose back-patching this?  You could argue that this is a bug in
   > 9.1 and 9.2.   Before that, they generate deprecation warnings, but do not
   > give the wrong answer.
   
   I think that backpatching to 9.1 would be reasonable, though I won't
   complain if the fix is only applied to HEAD.
   
   > If it is only to be applied to HEAD, or only to 9.1, 9.2, and HEAD, then
   > this part seems to be unnecessary and I think should be removed (setting a
   > value to its default is more likely to cause confusion than remove
   > confusion):
   >
   > SET standard_conforming_strings TO on;
   >
   > and the corresponding reset as well.
   
   Well, it may be unnecessary for people who use the modern default
   standard_conforming_strings. But some people have kept
   standard_conforming_strings=off in recent versions because they have
   old code which depends on this. And besides, it seems prudent to make
   the dependency explicit, rather than just hoping the user has the
   correct setting, and silently giving wrong results if not. Plus being
   able to work with old server versions.
   
   > Other than that, it does what it says (fix a broken example), does it
   > cleanly, we want this, and I have no other concerns.
   >
   > I guess the src/tutorial directory could participate in regression tests, in
   > which case this problem would have been detected when introduced, but I
   > don't think I can demand that you invent regression tests for a feature you
   > are just fixing rather than creating.
   
   Yeah, I don't think tying into the regression tests is warranted here.
   
   Josh
   
   
   

4. Re: string escaping in tutorial/syscat.source

   Jeff Janes <jeff.janes@gmail.com> — 2013-01-17T01:20:37Z

   On Tuesday, January 15, 2013, Josh Kupershmidt wrote:
   
   > On Tue, Jan 15, 2013 at 6:35 PM, Jeff Janes <jeff.janes@gmail.com<javascript:;>>
   > wrote:
   >
   > > Do you propose back-patching this?  You could argue that this is a bug in
   > > 9.1 and 9.2.   Before that, they generate deprecation warnings, but do
   > not
   > > give the wrong answer.
   >
   > I think that backpatching to 9.1 would be reasonable, though I won't
   > complain if the fix is only applied to HEAD.
   >
   > > If it is only to be applied to HEAD, or only to 9.1, 9.2, and HEAD, then
   > > this part seems to be unnecessary and I think should be removed (setting
   > a
   > > value to its default is more likely to cause confusion than remove
   > > confusion):
   > >
   > > SET standard_conforming_strings TO on;
   > >
   > > and the corresponding reset as well.
   >
   > Well, it may be unnecessary for people who use the modern default
   > standard_conforming_strings. But some people have kept
   > standard_conforming_strings=off in recent versions because they have
   > old code which depends on this.
   
   
   OK, I didn't anticipate them doing that in their default (i.e.
   postgresql.conf) but of course they might do that.
   
   I've marked it ready for committer.
   
   Thanks,
   
   Jeff
   

5. Re: string escaping in tutorial/syscat.source

   Tom Lane <tgl@sss.pgh.pa.us> — 2013-01-19T22:23:19Z

   Josh Kupershmidt <schmiddy@gmail.com> writes:
   > It seems the queries in ./src/tutorial/syscat.source use string
   > escaping with the assumption that standard_conforming_strings is off,
   > and thus give wrong results with modern versions. A simple fix is
   > attached.
   
   I tweaked the comments a little bit and committed this as far back
   as 9.1.  Thanks!
   
   			regards, tom lane