Thread: 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
>
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,
Pennsylvania19026
Re: [HACKERS] Re: [GENERAL] Bug with sequences in 6.4.2
From
reedstrm@wallace.ece.rice.edu (Ross J. Reedstrom)
Date:
<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
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
> 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,
Pennsylvania19026
> 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,
Pennsylvania19026
Bruce Momjian wrote:
>
> Good idea. Done.
>
> test=> select nextval('"Aa"');
> nextval
> -------
> 3
> (1 row)
select currval('"Aa"');
?
Vadim
> 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,
Pennsylvania19026
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
> 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,
Pennsylvania19026