Re: [PATCHES] PL/Python patch for Universal Newline Support
Michael Fuhr <[EMAIL PROTECTED]> writes: > Here's a PL/Python patch that includes a few tests for Universal > Newline Support. Applied to HEAD and 8.0 branch. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] PL/Python patch for Universal Newline Support
Here's a PL/Python patch that includes a few tests for Universal
Newline Support. "gmake installcheck" in src/pl/plpython is
successful in HEAD on my Solaris 9 box running Python 2.4.1c2.
I ran the tests against an unpatched 8.0.1 server and they failed
as expected.
--
Michael Fuhr
http://www.fuhr.org/~mfuhr/
Index: src/pl/plpython/feature.expected
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/feature.expected,v
retrieving revision 1.9
diff -c -r1.9 feature.expected
*** src/pl/plpython/feature.expected30 Jun 2003 18:31:42 - 1.9
--- src/pl/plpython/feature.expected24 Mar 2005 04:08:59 -
***
*** 137,139
--- 137,157
(0 rows)
+ SELECT newline_lf();
+ newline_lf
+
+ 123
+ (1 row)
+
+ SELECT newline_cr();
+ newline_cr
+
+ 123
+ (1 row)
+
+ SELECT newline_crlf();
+ newline_crlf
+ --
+ 123
+ (1 row)
+
Index: src/pl/plpython/plpython.c
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/plpython.c,v
retrieving revision 1.58
diff -c -r1.58 plpython.c
*** src/pl/plpython/plpython.c 17 Dec 2004 02:14:48 - 1.58
--- src/pl/plpython/plpython.c 24 Mar 2005 04:09:00 -
***
*** 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++;
Index: src/pl/plpython/plpython_function.sql
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/plpython_function.sql,v
retrieving revision 1.6
diff -c -r1.6 plpython_function.sql
*** src/pl/plpython/plpython_function.sql 30 Jun 2003 18:31:42 -
1.6
--- src/pl/plpython/plpython_function.sql 24 Mar 2005 04:09:00 -
***
*** 306,308
--- 306,324
open(args[0],"w").write(args[1])
return "Wrote to file: %s" % args[0]
' LANGUAGE plpythonu;
+
+ --
+ -- Universal Newline Support
+ --
+
+ CREATE OR REPLACE FUNCTION newline_lf() RETURNS integer AS
+ 'x = 100\ny = 23\nreturn x + y\n'
+ LANGUAGE plpythonu;
+
+ CREATE OR REPLACE FUNCTION newline_cr() RETURNS integer AS
+ 'x = 100\ry = 23\rreturn x + y\r'
+ LANGUAGE plpythonu;
+
+ CREATE OR REPLACE FUNCTION newline_crlf() RETURNS integer AS
+ 'x = 100\r\ny = 23\r\nreturn x + y\r\n'
+ LANGUAGE plpythonu;
Index: src/pl/plpython/plpython_test.sql
===
RCS file: /projects/cvsroot/pgsql/src/pl/plpython/plpython_test.sql,v
retrieving revision 1.2
diff -c -r1.2 plpython_test.sql
*** src/pl/plpython/plpython_test.sql 12 May 2001 17:49:32 - 1.2
--- src/pl/plpython/plpython_test.sql 24 Mar 2005 04:09:00 -
***
*** 61,63
--- 61,70
-- error in trigger
--
+ --
+ -- Check Universal Newline Support
+ --
+
+ SELECT newline_lf();
+ SELECT newline_cr();
+ SELECT newline_crlf();
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] PL/Python patch for Universal Newline Support
Michael Fuhr <[EMAIL PROTECTED]> writes: > 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? Yeah, that should work. I was too tired to think of that last night ;-) regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] PL/Python patch for Universal Newline Support
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
Re: [PATCHES] PL/Python patch for Universal Newline Support
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
Re: [PATCHES] PL/Python patch for Universal Newline Support
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-patches/2005-03/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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] PL/Python patch for Universal Newline Support
Michael Fuhr <[EMAIL PROTECTED]> 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
Re: [PATCHES] PL/Python patch for Universal Newline Support
On Mon, Mar 21, 2005 at 02:50:47AM -0500, Tom Lane wrote:
> Michael Fuhr <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
Re: [PATCHES] PL/Python patch for Universal Newline Support
Michael Fuhr <[EMAIL PROTECTED]> 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
Re: [PATCHES] PL/Python patch for Universal Newline Support
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
00061 0a 62 0a 63 0a 0a
007
(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-general/2005-03/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 [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] PL/Python patch for Universal Newline Support
Neil Conway <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] PL/Python patch for Universal Newline Support
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 - 1.58
--- src/pl/plpython/plpython.c 19 Mar 2005 04:29:55 -
***
*** 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
