Greetings,
there seems to be a slight bug in the parsing of the
nextval function, tested under 6.4.2.
It affects also the SERIAL type.

Symptom :

CREATE SEQUENCE "AA";
-- Correct, quoted identifier is allowed;
SELECT NEXTVAL('AA');
--Produces Error
--aa.nextval: sequence does not exist


Probable source of problem, the Argument to nextval is
not handled correctly as an Table Identifier.

E.g. nextval('"AA"') is generates 
"aa".nextval: sequence does not exist

Note the lowercase between the quotes.

I quickly browsed the sources, but have not found the
place where the conversion to lowercase occurs.

Please check this against 6.5 and 6.4.3 too.

With Regards,
	Stefan Wehner



Thu, 11 Mar 1999 14:22:12 +0100 (MET)
wehner@digital-security.com (Stefan Wehner)
Bug with sequences in 6.4.2
> Greetings,
> there seems to be a slight bug in the parsing of the
> nextval function, tested under 6.4.2.
> It affects also the SERIAL type.
> 
> Symptom :
> 
> CREATE SEQUENCE "AA";
> -- Correct, quoted identifier is allowed;
> SELECT NEXTVAL('AA');
> --Produces Error
> --aa.nextval: sequence does not exist
> 

Let me comment on this.    In the first statement, "AA" is used in an
SQL command, and we handle this correctly.  In the second case, NEXTVAL
is a function, called with a string.

Now in parse_func.c, we convert 'AA' to lower to try and find the
sequence table.  My assumption is that we should attempt to find the
table without doing a lower(), and if that fails, try lower.

Does that make sense to people.  We can't just lower it in every case.



> 
> Probable source of problem, the Argument to nextval is
> not handled correctly as an Table Identifier.
> 
> E.g. nextval('"AA"') is generates 
> "aa".nextval: sequence does not exist
> 
> Note the lowercase between the quotes.
> 
> I quickly browsed the sources, but have not found the
> place where the conversion to lowercase occurs.
> 
> Please check this against 6.5 and 6.4.3 too.
> 
> With Regards,
> 	Stefan Wehner
> 
> 
> 
> 


-- 
  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

Mon, 15 Mar 1999 10:55:53 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [GENERAL] Bug with sequences in 6.4.2

<problem with case of sequence name snipped>

> Now in parse_func.c, we convert 'AA' to lower to try and find the
> sequence table.  My assumption is that we should attempt to find the
> table without doing a lower(), and if that fails, try lower.
> 
> Does that make sense to people.  We can't just lower it in every case.
> 

Sounds line exactly the right fix to me. Oh, and a quick scan for other
cases of this assumption (auto lower() of field and attribute names) would
be nice, too.

Ross

-- 
Ross J. Reedstrom, Ph.D., <reedstrm@rice.edu> 
NSBRI Research Scientist/Programmer
Computer and Information Technology Institute
Rice University, 6100 S. Main St.,  Houston, TX 77005

Mon, 15 Mar 1999 10:46:14 -0600 (CST)
reedstrm@wallace.ece.rice.edu (Ross J. Reedstrom)
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
> Greetings,
> there seems to be a slight bug in the parsing of the
> nextval function, tested under 6.4.2.
> It affects also the SERIAL type.
> 
> Symptom :
> 
> CREATE SEQUENCE "AA";
> -- Correct, quoted identifier is allowed;
> SELECT NEXTVAL('AA');
> --Produces Error
> --aa.nextval: sequence does not exist
> 
> 
> Probable source of problem, the Argument to nextval is
> not handled correctly as an Table Identifier.
> 
> E.g. nextval('"AA"') is generates 
> "aa".nextval: sequence does not exist
> 
> Note the lowercase between the quotes.
> 
> I quickly browsed the sources, but have not found the
> place where the conversion to lowercase occurs.

OK, I have made the following change to allow case-sensitive handling of
nextval. It tries the exact case first, then tries lowercase.

-- 
  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

Index: parse_func.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/parser/parse_func.c,v
retrieving revision 1.39
retrieving revision 1.40
diff -c -r1.39 -r1.40
*** parse_func.c	1999/02/23 07:51:53	1.39
--- parse_func.c	1999/03/15 16:48:34	1.40
***************
*** 7,13 ****
   *
   *
   * IDENTIFICATION
!  *	  $Header: /usr/local/cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.39 1999/02/23 07:51:53 thomas Exp $
   *
   *-------------------------------------------------------------------------
   */
--- 7,13 ----
   *
   *
   * IDENTIFICATION
!  *	  $Header: /usr/local/cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.40 1999/03/15 16:48:34 momjian Exp $
   *
   *-------------------------------------------------------------------------
   */
***************
*** 21,26 ****
--- 21,27 ----
  #include "access/relscan.h"
  #include "access/sdir.h"
  #include "catalog/catname.h"
+ #include "catalog/heap.h"
  #include "catalog/indexing.h"
  #include "catalog/pg_inherits.h"
  #include "catalog/pg_proc.h"
***************
*** 440,446 ****
  
  		if (nodeTag(pair) == T_Ident && ((Ident *) pair)->isRel)
  		{
- 
  			/*
  			 * a relation
  			 */
--- 441,446 ----
***************
*** 573,588 ****
  		char	   *seqrel;
  		text	   *seqname;
  		int32		aclcheck_result = -1;
- 		extern text *lower(text *string);
  
  		Assert(length(fargs) == ((funcid == F_SETVAL) ? 2 : 1));
  		seq = (Const *) lfirst(fargs);
  		if (!IsA((Node *) seq, Const))
  			elog(ERROR, "Only constant sequence names are acceptable for function '%s'", funcname);
! 		seqname = lower((text *) DatumGetPointer(seq->constvalue));
! 		pfree(DatumGetPointer(seq->constvalue));
! 		seq->constvalue = PointerGetDatum(seqname);
! 		seqrel = textout(seqname);
  
  		if ((aclcheck_result = pg_aclcheck(seqrel, GetPgUserName(),
  					   (((funcid == F_NEXTVAL) || (funcid == F_SETVAL)) ?
--- 573,593 ----
  		char	   *seqrel;
  		text	   *seqname;
  		int32		aclcheck_result = -1;
  
  		Assert(length(fargs) == ((funcid == F_SETVAL) ? 2 : 1));
  		seq = (Const *) lfirst(fargs);
  		if (!IsA((Node *) seq, Const))
  			elog(ERROR, "Only constant sequence names are acceptable for function '%s'", funcname);
! 
! 		seqrel = textout((text *) DatumGetPointer(seq->constvalue));
! 		if (RelnameFindRelid(seqrel) == InvalidOid)
! 		{
! 			pfree(seqrel);
! 			seqname = lower((text *) DatumGetPointer(seq->constvalue));
! 			pfree(DatumGetPointer(seq->constvalue));
! 			seq->constvalue = PointerGetDatum(seqname);
! 			seqrel = textout(seqname);
! 		}
  
  		if ((aclcheck_result = pg_aclcheck(seqrel, GetPgUserName(),
  					   (((funcid == F_NEXTVAL) || (funcid == F_SETVAL)) ?
Mon, 15 Mar 1999 11:50:34 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [GENERAL] Bug with sequences in 6.4.2
Bruce Momjian <maillist@candle.pha.pa.us> writes:
>> CREATE SEQUENCE "AA";
>> -- Correct, quoted identifier is allowed;
>> SELECT NEXTVAL('AA');
>> --Produces Error

> Let me comment on this.    In the first statement, "AA" is used in an
> SQL command, and we handle this correctly.  In the second case, NEXTVAL
> is a function, called with a string.
> Now in parse_func.c, we convert 'AA' to lower to try and find the
> sequence table.  My assumption is that we should attempt to find the
> table without doing a lower(), and if that fails, try lower.
> Does that make sense to people.  We can't just lower it in every case.

That would create an ambiguity that is better avoided.  I think nextval
ought to duplicate the parser's behavior --- if possible, actually call
the same routine the parser uses for looking up a sequence name.
I suggest that it operate like this:

(1)	nextval('AA')		operates on sequence aa

	AA is lowercased, same as unquoted AA would be by the parser.

(2)	nextval('"AA"')		operates on sequence AA

	Quoted "AA" is treated as AA, same as parser would do it.

This should be fully backward compatible with existing SQL code, since
the existing nextval() code implements case (1).  I doubt anyone has
tried putting double quotes into their nextval arguments, so adding
the case (2) behavior shouldn't break anything.

			regards, tom lane

Mon, 15 Mar 1999 22:33:52 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2 
> Bruce Momjian <maillist@candle.pha.pa.us> writes:
> >> CREATE SEQUENCE "AA";
> >> -- Correct, quoted identifier is allowed;
> >> SELECT NEXTVAL('AA');
> >> --Produces Error
> 
> > Let me comment on this.    In the first statement, "AA" is used in an
> > SQL command, and we handle this correctly.  In the second case, NEXTVAL
> > is a function, called with a string.
> > Now in parse_func.c, we convert 'AA' to lower to try and find the
> > sequence table.  My assumption is that we should attempt to find the
> > table without doing a lower(), and if that fails, try lower.
> > Does that make sense to people.  We can't just lower it in every case.
> 
> That would create an ambiguity that is better avoided.  I think nextval
> ought to duplicate the parser's behavior --- if possible, actually call
> the same routine the parser uses for looking up a sequence name.
> I suggest that it operate like this:
> 
> (1)	nextval('AA')		operates on sequence aa
> 
> 	AA is lowercased, same as unquoted AA would be by the parser.
> 
> (2)	nextval('"AA"')		operates on sequence AA
> 
> 	Quoted "AA" is treated as AA, same as parser would do it.
> 
> This should be fully backward compatible with existing SQL code, since
> the existing nextval() code implements case (1).  I doubt anyone has
> tried putting double quotes into their nextval arguments, so adding
> the case (2) behavior shouldn't break anything.

I can do that.  It looked kind of strange.  Usually they do:

	create sequence "Aa";

Because nextval is a function with a parameter, it would be
nextval('"Aa"').  I don't think we want to do this for all function
parameters, but just for nextval.

I can do that.

-- 
  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

Mon, 15 Mar 1999 23:07:47 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
> That would create an ambiguity that is better avoided.  I think nextval
> ought to duplicate the parser's behavior --- if possible, actually call
> the same routine the parser uses for looking up a sequence name.
> I suggest that it operate like this:
> 
> (1)	nextval('AA')		operates on sequence aa
> 
> 	AA is lowercased, same as unquoted AA would be by the parser.
> 
> (2)	nextval('"AA"')		operates on sequence AA
> 
> 	Quoted "AA" is treated as AA, same as parser would do it.
> 
> This should be fully backward compatible with existing SQL code, since
> the existing nextval() code implements case (1).  I doubt anyone has
> tried putting double quotes into their nextval arguments, so adding
> the case (2) behavior shouldn't break anything.

Good idea.  Done.

test=> select nextval('"Aa"');
nextval
-------
      3
(1 row)



-- 
  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

Mon, 15 Mar 1999 23:32:18 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
Bruce Momjian wrote:
> 
> Good idea.  Done.
> 
> test=> select nextval('"Aa"');
> nextval
> -------
>       3
> (1 row)

select currval('"Aa"');

?

Vadim

Tue, 16 Mar 1999 11:39:55 +0700
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
> Bruce Momjian wrote:
> > 
> > Good idea.  Done.
> > 
> > test=> select nextval('"Aa"');
> > nextval
> > -------
> >       3
> > (1 row)
> 
> select currval('"Aa"');

Someone complained they could not get a sequence named "Aa" because the
code auto-lowercased the nextval parameter.  The new code accepts
nextval('"Aa"'), and preserves the case.  Other cases are
auto-lowercased.

-- 
  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

Mon, 15 Mar 1999 23:50:17 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
Bruce Momjian wrote:
> 
> > select currval('"Aa"');
> 
> Someone complained they could not get a sequence named "Aa" because the
> code auto-lowercased the nextval parameter.  The new code accepts
> nextval('"Aa"'), and preserves the case.  Other cases are
> auto-lowercased.

No, just looked in new code and seems that your changes in 
parse_func.c handle currval() and setval() as well, sorry.

Vadim

Tue, 16 Mar 1999 11:54:25 +0700
Vadim Mikheev <vadim@krs.ru>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
> Bruce Momjian wrote:
> > 
> > > select currval('"Aa"');
> > 
> > Someone complained they could not get a sequence named "Aa" because the
> > code auto-lowercased the nextval parameter.  The new code accepts
> > nextval('"Aa"'), and preserves the case.  Other cases are
> > auto-lowercased.
> 
> No, just looked in new code and seems that your changes in 
> parse_func.c handle currval() and setval() as well, sorry.

I meant that non-double quoted cases are auto-lowered.  I didn't realize
I was also dealing with other *val cases.  I guess that is good.

I am on IRC 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

Mon, 15 Mar 1999 23:57:04 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2