The attached patch mimics Universal Newline Support for PL/Python
functions. For background, see PEP 278 and the recent "plpython
function problem workaround" thread in pgsql-general:

http://www.python.org/peps/pep-0278.html
http://archives.postgresql.org/pgsql...3/msg00823.php

In short, embedded Python code must have lines that end in \n, not
\r\n or \r. This patch adds logic to PLy_procedure_munge_source()
to skip \r followed by \n, otherwise to convert \r to \n. I tested
it in HEAD but REL8_0_STABLE has the same version of plpython.c (1.58),
so it should work there as well.

Should the PL/Python documentation mention this behavior? If so
then I can resubmit with a documentation patch.

How should I submit regression tests? Here's what I did:

CREATE FUNCTION test_lf() RETURNS integer AS
'x=100\ny=23\nreturn x+y\n'
LANGUAGE plpythonu;

CREATE FUNCTION test_crlf() RETURNS integer AS
'x=100\r\ny=23\r\nreturn x+y\r\n'
LANGUAGE plpythonu;

CREATE FUNCTION test_cr() RETURNS integer AS
'x=100\ry=23\rreturn x+y\r'
LANGUAGE plpythonu;

Here's what an unpatched system does:

SELECT test_lf();
test_lf
---------
123
(1 row)

SELECT test_cr();
ERROR: plpython: could not compile function "test_cr"
DETAIL: exceptions.SyntaxError: invalid syntax (line 2)

SELECT test_crlf();
ERROR: plpython: could not compile function "test_crlf"
DETAIL: exceptions.SyntaxError: invalid syntax (line 2)

Here's a patched system:

SELECT test_lf();
test_lf
---------
123
(1 row)

SELECT test_cr();
test_cr
---------
123
(1 row)

SELECT test_crlf();
test_crlf
-----------
123
(1 row)

This patch doesn't address the indentation problem mentioned here:

http://archives.postgresql.org/pgsql...3/msg00762.php

The solution to that problem is still being discussed.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/


---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

Michael Fuhr wrote:
> Should the PL/Python documentation mention this behavior?

Isn't this the behavior the user would expect? If so, I guess it's okay
not to document it.

> How should I submit regression tests?

Yes, please.

> *** src/pl/plpython/plpython.c 17 Dec 2004 02:14:48 -0000 1.58
> --- src/pl/plpython/plpython.c 19 Mar 2005 04:29:55 -0000
> ***************
> *** 1206,1215 ****
>
> while (*sp != '\0')
> {
> ! if (*sp == '\n')
> {
> ! *mp++ = *sp++;
> *mp++ = '\t';
> }
> else
> *mp++ = *sp++;
> --- 1206,1219 ----
>
> while (*sp != '\0')
> {
> ! if (*sp == '\r' && *(sp + 1) == '\n')
> ! sp++;
> !
> ! if (*sp == '\n' || *sp == '\r')
> {
> ! *mp++ = '\n';
> *mp++ = '\t';
> + sp++;
> }
> else
> *mp++ = *sp++;

Does this work for "\r\n" embedded in string literals?

-Neil

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

Neil Conway <neilc@samurai.com> writes:
> Does this work for "\r\n" embedded in string literals?

I believe we'd concluded that Python will unconditionally convert all
\r\n to \n when reading any text file --- including script files ---
and therefore that's what Python programmers will expect to have happen
to scripts. In other words we should deliberately be non-syntax-aware
when stripping \r.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

On Mon, Mar 21, 2005 at 10:28:28AM +1100, Neil Conway wrote:
> Michael Fuhr wrote:
>
> >How should I submit regression tests?
>
> Yes, please.

The operative word there was "how" :-) I don't see anything testing
PL/{Python,Perl,Tcl} under src/test/regress -- should I put something
there? Can regression tests be run conditionally? If so, what's
the preferred way to do that? If regression tests for this kind
of patch need to be done another way, how should I submit them?

> Does this work for "\r\n" embedded in string literals?

Literal carriage returns (ASCII character 13) will be translated,
just as Python would do when reading a script from a file if Python
is built with Universal Newline Support (enabled by default, at
least in recent versions). Escape sequences won't be touched if
they're stored in prosrc as escape sequences (backslash-r) instead
of actual carriage returns. For example:

CREATE FUNCTION foo() RETURNS text AS $$
return "\r\n"
$$ LANGUAGE plpythonu IMMUTABLE;

SELECT prosrc FROM pg_proc WHERE proname = 'foo';
prosrc
-----------------

return "\r\n"

(1 row)

SELECT length(foo()), ascii(foo()), ascii(substr(foo(), 2));
length | ascii | ascii
--------+-------+-------
2 | 13 | 10
(1 row)

But the following fails (the function is in single quotes instead
of dollar quotes, so \r\n becomes an actual CRLF):

CREATE OR REPLACE FUNCTION foo() RETURNS text AS '
return "\r\n"
' LANGUAGE plpythonu IMMUTABLE;

SELECT prosrc FROM pg_proc WHERE proname = 'foo';
prosrc
---------------

return "
"

(1 row)

SELECT foo();
ERROR: plpython: could not compile function "foo"
DETAIL: exceptions.SyntaxError: EOL while scanning single-quoted string (line 3)

An ordinary Python script that looked like that would fail as well:

% cat -v foo.py
print "^M
"

% python foo.py
File "foo.py", line 1
print "
^
SyntaxError: EOL while scanning single-quoted string

Note Python's translation in this case:

% cat -v foo.py
print """a^M
b^M
c^M
"""

% python foo.py | od -tx1
0000000 61 0a 62 0a 63 0a 0a
0000007

(The extra newline is appended by "print".)

I was thinking that the patch could be applied to HEAD and hopefully
somebody could do some additional testing with Windows clients and
servers. If there are no problems, and especially if the patch
solves the problem it's intended to solve, then the patch could be
applied to REL8_0_STABLE so it would be in 8.0.2 whenever that comes
out. Tom, that sounded reasonable to you, didn't it?

http://archives.postgresql.org/pgsql...3/msg00842.php

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Michael Fuhr <mike@fuhr.org> writes:
> The operative word there was "how" :-) I don't see anything testing
> PL/{Python,Perl,Tcl} under src/test/regress -- should I put something
> there?

No.

The PLs have their own regression tests in their individual src/pl
directories; feel free to hack on those (most are pretty lame :-()

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

On Mon, Mar 21, 2005 at 02:50:47AM -0500, Tom Lane wrote:
> Michael Fuhr <mike@fuhr.org> writes:
> > The operative word there was "how" :-) I don't see anything testing
> > PL/{Python,Perl,Tcl} under src/test/regress -- should I put something
> > there?
>
> No.
>
> The PLs have their own regression tests in their individual src/pl
> directories; feel free to hack on those (most are pretty lame :-()

Thanks -- I'll add some tests there and resubmit.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

Michael Fuhr <mike@fuhr.org> writes:
> On Mon, Mar 21, 2005 at 02:50:47AM -0500, Tom Lane wrote:
>> The PLs have their own regression tests in their individual src/pl
>> directories; feel free to hack on those (most are pretty lame :-()

> Thanks -- I'll add some tests there and resubmit.

One thing that needs some thought is how you are going to test this
robustly. I'd not feel any great deal of confidence in a test that
expects that we can push \r\n sequences into CVS and expect them to
survive unmodified into distributed filesets. Every so often someone
commits a DOS-newlines file into CVS, and they hear about it PDQ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

On Mon, Mar 21, 2005 at 03:02:57AM -0500, Tom Lane wrote:
>
> One thing that needs some thought is how you are going to test this
> robustly. I'd not feel any great deal of confidence in a test that
> expects that we can push \r\n sequences into CVS and expect them to
> survive unmodified into distributed filesets. Every so often someone
> commits a DOS-newlines file into CVS, and they hear about it PDQ...

We're testing what happens when \r gets into prosrc, right? Shouldn't
creating the function with \r in single quotes instead of dollar
quotes work, as in the tests I showed in my original submission?

http://archives.postgresql.org/pgsql...3/msg00186.php

The functions with \r failed as expected in an unpatched system,
and they worked in a patched system. The CREATE FUNCTION statements
used \r escape sequences, so that shouldn't be a problem with CVS
since they're not actual carriage returns in the SQL, right?

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

On Mon, Mar 21, 2005 at 01:19:35AM -0700, Michael Fuhr wrote:

> We're testing what happens when \r gets into prosrc, right? Shouldn't
> creating the function with \r in single quotes instead of dollar
> quotes work, as in the tests I showed in my original submission?

....or what about a little script or C program that generates CREATE
FUNCTION statements with various line endings? I suppose that would
more accurately simulate what client apps are doing.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Michael Fuhr said:
> On Mon, Mar 21, 2005 at 03:02:57AM -0500, Tom Lane wrote:
>>
>> One thing that needs some thought is how you are going to test this
>> robustly. I'd not feel any great deal of confidence in a test that
>> expects that we can push \r\n sequences into CVS and expect them to
>> survive unmodified into distributed filesets. Every so often someone
>> commits a DOS-newlines file into CVS, and they hear about it PDQ...
>
> We're testing what happens when \r gets into prosrc, right? Shouldn't
> creating the function with \r in single quotes instead of dollar
> quotes work, as in the tests I showed in my original submission?
>

Yes - also worked in testing CSV multiline support - see copy regression
test.
cheers

andrew



---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend