> > tested your patch and there was no change in result. I think it 
> > wouldn't be nice if this will point out a bug in the perl pg driver 
> > because I can't imagine that you would like to do such things in 
> > there ...
> > 
> > the new crash-me tests results are sent to monty so I think he will 
> > put them online tomorrow (today for you I think). I also did a test 
> > run on oracle and on a microsoft sql 7 server on windows nt (oracle 
> > on linux).
> 
> Enclosed is a patch that shows our perl interface can't handle '--'
> comments, even though psql and the backend directly can handle them.
> 
> To add complexity to this, the backend -d3 log from the perl test
> session shows the same query that works perfectly in a direct backend
> connection.
> 
> Can anyone suggest a cause for this?

OK, fix attached.  Seems our "--" comments required a newline on the
end, which was not being done in interfaces like Perl.  Added a test in
the perl code for the trailing comments, and patched scan.l.

Seems this should only be applied to 6.6.  Applied to 6.6.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

? doc/src/sgml/install.htm
? src/log
? src/config.log
? src/config.cache
? src/config.status
? src/GNUmakefile
? src/Makefile.global
? src/Makefile.custom
? src/backend/fmgr.h
? src/backend/parse.h
? src/backend/postgres
? src/backend/global1.bki.source
? src/backend/local1_template1.bki.source
? src/backend/global1.description
? src/backend/local1_template1.description
? src/backend/bootstrap/bootparse.c
? src/backend/bootstrap/bootstrap_tokens.h
? src/backend/bootstrap/bootscanner.c
? src/backend/catalog/genbki.sh
? src/backend/catalog/global1.bki.source
? src/backend/catalog/global1.description
? src/backend/catalog/local1_template1.bki.source
? src/backend/catalog/local1_template1.description
? src/backend/port/Makefile
? src/backend/utils/Gen_fmgrtab.sh
? src/backend/utils/fmgr.h
? src/backend/utils/fmgrtab.c
? src/bin/cleardbdir/cleardbdir
? src/bin/createdb/createdb
? src/bin/createlang/createlang
? src/bin/createuser/createuser
? src/bin/destroydb/destroydb
? src/bin/destroylang/destroylang
? src/bin/destroyuser/destroyuser
? src/bin/initdb/initdb
? src/bin/initlocation/initlocation
? src/bin/ipcclean/ipcclean
? src/bin/pg_dump/Makefile
? src/bin/pg_dump/pg_dump
? src/bin/pg_id/pg_id
? src/bin/pg_passwd/pg_passwd
? src/bin/pg_version/Makefile
? src/bin/pg_version/pg_version
? src/bin/pgtclsh/mkMakefile.tcldefs.sh
? src/bin/pgtclsh/mkMakefile.tkdefs.sh
? src/bin/pgtclsh/Makefile.tkdefs
? src/bin/pgtclsh/Makefile.tcldefs
? src/bin/pgtclsh/pgtclsh
? src/bin/pgtclsh/pgtksh
? src/bin/psql/Makefile
? src/bin/psql/psql
? src/include/version.h
? src/include/config.h
? src/interfaces/ecpg/lib/Makefile
? src/interfaces/ecpg/lib/libecpg.so.3.0.1
? src/interfaces/ecpg/lib/libecpg.so.3.0.3
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpgtcl/Makefile
? src/interfaces/libpgtcl/libpgtcl.so.2.0
? src/interfaces/libpq/Makefile
? src/interfaces/libpq/libpq.so.2.0
? src/interfaces/libpq++/Makefile
? src/interfaces/libpq++/libpq++.so.3.0
? src/interfaces/odbc/GNUmakefile
? src/interfaces/odbc/Makefile.global
? src/interfaces/perl5/blib
? src/interfaces/perl5/pm_to_blib
? src/interfaces/perl5/Pg.c
? src/interfaces/perl5/Pg.bs
? src/interfaces/perl5/Makefile
? src/lextest/lex.yy.c
? src/lextest/lextest
? src/pl/plpgsql/src/Makefile
? src/pl/plpgsql/src/mklang.sql
? src/pl/plpgsql/src/pl_gram.c
? src/pl/plpgsql/src/pl.tab.h
? src/pl/plpgsql/src/pl_scan.c
? src/pl/plpgsql/src/libplpgsql.so.1.0
? src/pl/tcl/mkMakefile.tcldefs.sh
? src/pl/tcl/Makefile.tcldefs
? src/test/regress/regress.out
? src/test/regress/regression.diffs
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/constraints.out
? src/test/regress/expected/install_plpgsql.out
? src/test/regress/results/boolean.out
? src/test/regress/results/char.out
? src/test/regress/results/name.out
? src/test/regress/results/varchar.out
? src/test/regress/results/text.out
? src/test/regress/results/strings.out
? src/test/regress/results/int2.out
? src/test/regress/results/int4.out
? src/test/regress/results/int8.out
? src/test/regress/results/oid.out
? src/test/regress/results/float4.out
? src/test/regress/results/float8.out
? src/test/regress/results/numerology.out
? src/test/regress/results/point.out
? src/test/regress/results/lseg.out
? src/test/regress/results/box.out
? src/test/regress/results/path.out
? src/test/regress/results/polygon.out
? src/test/regress/results/circle.out
? src/test/regress/results/geometry.out
? src/test/regress/results/timespan.out
? src/test/regress/results/datetime.out
? src/test/regress/results/reltime.out
? src/test/regress/results/abstime.out
? src/test/regress/results/tinterval.out
? src/test/regress/results/horology.out
? src/test/regress/results/inet.out
? src/test/regress/results/comments.out
? src/test/regress/results/oidjoins.out
? src/test/regress/results/type_sanity.out
? src/test/regress/results/opr_sanity.out
? src/test/regress/results/create_function_1.out
? src/test/regress/results/create_type.out
? src/test/regress/results/create_table.out
? src/test/regress/results/create_function_2.out
? src/test/regress/results/constraints.out
? src/test/regress/results/triggers.out
? src/test/regress/results/copy.out
? src/test/regress/results/onek.data
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/constraints.sql
? src/test/regress/sql/install_plpgsql.sql
? src/tools/backend/flow.eps
? src/tools/backend/flow.ps
? src/tools/backend/flow.png
? src/tools/backend/flow.tif
Index: src/backend/parser/scan.l
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/parser/scan.l,v
retrieving revision 1.57
diff -c -r1.57 scan.l
*** src/backend/parser/scan.l	1999/09/28 03:41:36	1.57
--- src/backend/parser/scan.l	1999/10/08 04:58:23
***************
*** 167,173 ****
  
  param			\${integer}
  
! comment			("--"|"//").*\n
  
  space			[ \t\n\f]
  other			.
--- 167,173 ----
  
  param			\${integer}
  
! comment			("--"|"//").*
  
  space			[ \t\n\f]
  other			.
Index: src/interfaces/perl5/test.pl
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/interfaces/perl5/test.pl,v
retrieving revision 1.9
diff -c -r1.9 test.pl
*** src/interfaces/perl5/test.pl	1998/09/27 19:12:26	1.9
--- src/interfaces/perl5/test.pl	1999/10/08 04:58:30
***************
*** 147,153 ****
  
  ######################### create and insert into table
  
! $result = $conn->exec("CREATE TABLE person (id int4, name char(16))");
  die $conn->errorMessage unless PGRES_COMMAND_OK eq $result->resultStatus;
  my $cmd = $result->cmdStatus;
  ( "CREATE" eq $cmd )
--- 147,153 ----
  
  ######################### create and insert into table
  
! $result = $conn->exec("CREATE TABLE person (id int4, name char(16)) -- test");
  die $conn->errorMessage unless PGRES_COMMAND_OK eq $result->resultStatus;
  my $cmd = $result->cmdStatus;
  ( "CREATE" eq $cmd )
Fri, 8 Oct 1999 01:03:22 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [GENERAL] Re: [PHP3] Re: PostgreSQL vs Mysql comparison
Bruce Momjian <maillist@candle.pha.pa.us> writes:
> *** src/backend/parser/scan.l	1999/09/28 03:41:36	1.57
> --- src/backend/parser/scan.l	1999/10/08 04:58:23
> ***************
> *** 167,173 ****
>  
>   param			\${integer}
>  
> ! comment			("--"|"//").*\n
>  
>   space			[ \t\n\f]
>   other			.
> --- 167,173 ----
>  
>   param			\${integer}
>  
> ! comment			("--"|"//").*
>  
>   space			[ \t\n\f]
>   other			.

Ah, so the problem was that the perl interface didn't append a newline?
Good catch.  I don't like this fix, however, since I fear it will
alter behavior for the case where there is an embedded newline in the
query buffer.  For example
	CREATE TABLE mytab -- comment \n (f1 int)
can be sent to the backend as one string (though not via psql).  With
the above change in scan.l I think the comment will be taken to include
everything from -- to the end of the buffer, which is wrong.

A better solution IMHO is to leave scan.l as it was and instead
always append a \n to the presented query string before we parse.

BTW, might be a good idea to add \r to that list of "space" characters
so we don't mess up on DOS-style newlines (\r\n).

			regards, tom lane

Fri, 08 Oct 1999 09:47:03 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: [GENERAL] Re: [PHP3] Re: PostgreSQL vs Mysql
	comparison
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> > *** src/backend/parser/scan.l	1999/09/28 03:41:36	1.57
> > --- src/backend/parser/scan.l	1999/10/08 04:58:23
> > ***************
> > *** 167,173 ****
> >  
> >   param			\${integer}
> >  
> > ! comment			("--"|"//").*\n
> >  
> >   space			[ \t\n\f]
> >   other			.
> > --- 167,173 ----
> >  
> >   param			\${integer}
> >  
> > ! comment			("--"|"//").*
> >  
> >   space			[ \t\n\f]
> >   other			.
> 
> Ah, so the problem was that the perl interface didn't append a newline?
> Good catch.  I don't like this fix, however, since I fear it will
> alter behavior for the case where there is an embedded newline in the
> query buffer.  For example
> 	CREATE TABLE mytab -- comment \n (f1 int)

No problem.  I just added test code to see if it works, and it does:

	$result = $conn->exec(
	"CREATE TABLE person (id int4, -- test\n name char(16)) -- test"); 

Tests embedded newline, and comment without newline.

I will commit this so it will always be tested by the perl test code.


> can be sent to the backend as one string (though not via psql).  With
> the above change in scan.l I think the comment will be taken to include
> everything from -- to the end of the buffer, which is wrong.

No, seems lex only goes the end-of-line unless you specifically say \n.

> 
> A better solution IMHO is to leave scan.l as it was and instead
> always append a \n to the presented query string before we parse.

Problem here is that perl is not the only interface that would have this
problem.  In fact, I am not sure why libpq doesn't have this problem. 
Maybe it does.  Anyway, changing all the interfaces would be a pain, and
non-portable to older releases.

> 
> BTW, might be a good idea to add \r to that list of "space" characters
> so we don't mess up on DOS-style newlines (\r\n).

Interesting idea.  I tried that, but the problem is things like this:

	xqliteral       [\\](.|\n)

If I change it to:

	xqliteral       [\\](.|\n|\r)

then \r\n is not going to work, and if I change it to:

	xqliteral       [\\](.|\n|\r)+

Then \n\n is going to be accepted when it shouldn't.  Seems I will have
to leave it alone for now.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 8 Oct 1999 12:30:21 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: [GENERAL] Re: [PHP3] Re: PostgreSQL vs Mysql
	comparison
Bruce Momjian <maillist@candle.pha.pa.us> writes:
>> Ah, so the problem was that the perl interface didn't append a newline?
>> Good catch.  I don't like this fix, however, since I fear it will
>> alter behavior for the case where there is an embedded newline in the
>> query buffer.

> I will commit this so it will always be tested by the perl test code.

But how often do we run that?

> No, seems lex only goes the end-of-line unless you specifically say \n.

OK, I see in the flex manual that "." matches everything except newline,
so I guess it will work.  At least with flex.  But ".*" patterns with
no clearly defined terminator always make me itch --- it doesn't take
much change to get the wrong result.

>> A better solution IMHO is to leave scan.l as it was and instead
>> always append a \n to the presented query string before we parse.

> Problem here is that perl is not the only interface that would have this
> problem.  In fact, I am not sure why libpq doesn't have this problem. 

No, I wasn't suggesting patching the perl interface; I was suggesting
changing the backend, ie, adding the \n to the received query in
postgres.c just before we hand it off to the parser.

>> BTW, might be a good idea to add \r to that list of "space" characters
>> so we don't mess up on DOS-style newlines (\r\n).

> Interesting idea.  I tried that, but the problem is things like this:
> 	xqliteral       [\\](.|\n)

Hmm, didn't think about what to do with \r inside literals.  I agree,
it's not worth trying to be smart about those, so I suppose ignoring
them outside literals would be inconsistent.  Still, how many people
try to enter newlines within literals?  Adding \r to the whitespace
set and nothing else might still be a useful compatibility gain.

			regards, tom lane

Fri, 08 Oct 1999 18:53:18 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: [GENERAL] Re: [PHP3] Re: PostgreSQL vs Mysql
	comparison
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> >> Ah, so the problem was that the perl interface didn't append a newline?
> >> Good catch.  I don't like this fix, however, since I fear it will
> >> alter behavior for the case where there is an embedded newline in the
> >> query buffer.
> 
> > I will commit this so it will always be tested by the perl test code.
> 
> But how often do we run that?

Well, at least it is there now, and I will do --with-perl here, so it
will be run.

> > No, seems lex only goes the end-of-line unless you specifically say \n.
> 
> OK, I see in the flex manual that "." matches everything except newline,
> so I guess it will work.  At least with flex.  But ".*" patterns with
> no clearly defined terminator always make me itch --- it doesn't take
> much change to get the wrong result.

True, but it fixes the problem.

> 
> >> A better solution IMHO is to leave scan.l as it was and instead
> >> always append a \n to the presented query string before we parse.
> 
> > Problem here is that perl is not the only interface that would have this
> > problem.  In fact, I am not sure why libpq doesn't have this problem. 
> 
> No, I wasn't suggesting patching the perl interface; I was suggesting
> changing the backend, ie, adding the \n to the received query in
> postgres.c just before we hand it off to the parser.

I try to avoid hacks like that if I can.  Removing \n from the comment
termination is much clearer and more limited.

> 
> >> BTW, might be a good idea to add \r to that list of "space" characters
> >> so we don't mess up on DOS-style newlines (\r\n).
> 
> > Interesting idea.  I tried that, but the problem is things like this:
> > 	xqliteral       [\\](.|\n)
> 
> Hmm, didn't think about what to do with \r inside literals.  I agree,
> it's not worth trying to be smart about those, so I suppose ignoring
> them outside literals would be inconsistent.  Still, how many people
> try to enter newlines within literals?  Adding \r to the whitespace
> set and nothing else might still be a useful compatibility gain.

Added \r to the {space} pattern.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Fri, 8 Oct 1999 21:34:43 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: [GENERAL] Re: [PHP3] Re: PostgreSQL vs Mysql
	comparison