Thread: psql misbehaves because of a simple typo
Hi,
Is it me (who hasn't read some FAQ or a doc/man page) or
it's a bug in the psql interactive terminal?
A sample session is provided at the bottom. I just typed
a simple CREATE TABLE command and did not put closing
parenthesis (I was typing too fast); I did put a semicolon, however.
psql gave me no error message whatsoever and accepted whatever
input afterwards and ignored it with the exception of \commands.
Was this reported? Do you need some other info?
Logs?
o I have a RHL7.1 and the tarball of 7.2b3 downloaded from the website.
o # ./configure --enable-nls --enable-multibyte --enable-locale --enable-debug --enable-cassert
Here is the session:
[regress72b3@gunn regress72b3]$ /usr/local/pgsql/bin/psql test
Welcome to psql, the PostgreSQL interactive terminal.
Type: \copyright for distribution terms \h for help with SQL commands \? for help on internal slash commands
\g or terminate with semicolon to execute query \q to quit
test=# create table test2(id serial;
test(# select version();
test(# ?
test(# \?\a toggle between unaligned and aligned output mode\c[onnect] [DBNAME|- [USER]]
connectto new database (currently "test")\C TITLE set table title\cd [DIRNAME] change the current working
directory\copy... perform SQL COPY with data stream to the client host\copyright show PostgreSQL usage and
distributionterms\d TABLE describe table (or view, index, sequence)\d{t|i|s|v}... list
tables/indexes/sequences/views\d{p|S|l} list access privileges, system tables, or large objects\da list
aggregatefunctions\dd NAME show comment for table, type, function, or operator\df list functions\do
list operators\dT list data types\e FILENAME edit the current query buffer or file with external
editor\echoTEXT write text to standard output\encoding ENCODING set client encoding\f STRING set field
separator\gFILENAME send SQL command to server (and write results to file or |pipe)\h NAME help on syntax of
SQLcommands, * for all commands\H toggle HTML output mode (currently off)\i FILENAME execute commands
fromfile\l list all databases\lo_export, \lo_import, \lo_list, \lo_unlink large object
operations\oFILENAME send all query results to file or |pipe\p show the content of the current query
buffer\psetVAR set table output option (VAR := {format|border|expanded|
fieldsep|null|recordsep|tuples_only|title|tableattr|pager})\q quit psql\qecho TEXT write text to query
outputstream (see \o)\r reset (clear) the query buffer\s FILENAME print history or save it to file\set
NAMEVALUE set internal variable\t show only rows (currently off)\T TEXT set HTML table tag
attributes\unsetNAME unset (delete) internal variable\w FILENAME write current query buffer to file\x
toggleexpanded output (currently off)\z list table access privileges\! [COMMAND] execute command in shell
orstart interactive shell
test-# select version();
ERROR: parser: parse error at or near ";"
test=# select version(); version
-------------------------------------------------------------PostgreSQL 7.2b3 on i686-pc-linux-gnu, compiled by GCC
2.96
(1 row)
test=#
--
Serguei A. Mokhov
"Serguei Mokhov" <sa_mokho@alcor.concordia.ca> writes:
> Is it me (who hasn't read some FAQ or a doc/man page) or
> it's a bug in the psql interactive terminal?
Both. There's a bug there, but it's not the one you think.
psql seems to forget that it's got unmatched parentheses in the
buffer after executing a \? command. Watch the prompt:
regression=# (select
regression(# \?... yadda yadda ...
regression-# 2;
ERROR: parser: parse error at or near ";"
regression=# (select
regression(# \?... yadda yadda ...
regression-# 2);?column?
---------- 2
(1 row)
In the first example, it should not have thought that it had
a complete command after "2;".
regards, tom lane
----- Original Message ----- From: Tom Lane <tgl@sss.pgh.pa.us> Sent: Sunday, December 02, 2001 10:16 AM > "Serguei Mokhov" <sa_mokho@alcor.concordia.ca> writes: > > Is it me (who hasn't read some FAQ or a doc/man page) or > > it's a bug in the psql interactive terminal? > > Both. Oops. > There's a bug there, but it's not the one you think. No, that's what I thought. > psql seems to forget that it's got unmatched parentheses in the > buffer after executing a \? command. Watch the prompt: ... and that's what I've witnessed. Sorry for the 'false' alarm. This feature just bumped into me unexpectedly. -s
> "Serguei Mokhov" <sa_mokho@alcor.concordia.ca> writes:
> > Is it me (who hasn't read some FAQ or a doc/man page) or
> > it's a bug in the psql interactive terminal?
>
> Both. There's a bug there, but it's not the one you think.
> psql seems to forget that it's got unmatched parentheses in the
> buffer after executing a \? command. Watch the prompt:
>
> regression=# (select
> regression(# \?
> ... yadda yadda ...
> regression-# 2;
> ERROR: parser: parse error at or near ";"
> regression=# (select
> regression(# \?
> ... yadda yadda ...
> regression-# 2);
> ?column?
> ----------
> 2
> (1 row)
>
> In the first example, it should not have thought that it had
> a complete command after "2;".
The actual code that resets the paren level is:
/* backslash command */else if (was_bslash){ const char *end_of_cmd = NULL;
paren_level = 0; line[i - prevlen] = '\0'; /* overwrites backslash */
I believe this code is good. If someone issues a backslash command,
they probably want to get out of they paren nesting, or may not even
know they are in paren nesting, as Serguei did not in the example shown.
While it doesn't seem logical, it does help prevent people from getting
stuck in psql. The fact that backslash commands inside parens clear the
counter is a minor anoyance but resonable behavior.
-- Bruce Momjian | http://candle.pha.pa.us pgman@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 <pgman@candle.pha.pa.us> writes:
> The fact that backslash commands inside parens clear the
> counter is a minor anoyance but resonable behavior.
If they cleared the command buffer too, it might be construed
as novice-friendly behavior (though I'd still call it broken).
However, letting the counter get out of sync with the buffer
contents cannot be called anything but a bug.
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > The fact that backslash commands inside parens clear the > > counter is a minor anoyance but resonable behavior. > > If they cleared the command buffer too, it might be construed > as novice-friendly behavior (though I'd still call it broken). > However, letting the counter get out of sync with the buffer > contents cannot be called anything but a bug. Maybe we should clear the counter _only_ if the user clears the buffer, or sends the query to the backend with \g. -- Bruce Momjian | http://candle.pha.pa.us pgman@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 <pgman@candle.pha.pa.us> writes: > > The fact that backslash commands inside parens clear the > > counter is a minor anoyance but resonable behavior. > > If they cleared the command buffer too, it might be construed > as novice-friendly behavior (though I'd still call it broken). > However, letting the counter get out of sync with the buffer > contents cannot be called anything but a bug. OK, so what do we want to do? Clearing the buffer on a any backslash command is clearly not what we want to do. Should we clear the buffer on a backslash command _only_ if the number of paren's is not even? If we don't clear the counter on a backslash command with uneven parens, do we risk trapping people in psql? -- Bruce Momjian | http://candle.pha.pa.us pgman@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 <pgman@candle.pha.pa.us> writes:
> OK, so what do we want to do? Clearing the buffer on a any backslash
> command is clearly not what we want to do. Should we clear the buffer
> on a backslash command _only_ if the number of paren's is not even? If
> we don't clear the counter on a backslash command with uneven parens, do
> we risk trapping people in psql?
"Trap"? AFAIK \q works in any case.
\r should reset both the buffer and the counter, and seems to do so,
though I'm not quite seeing where it manages to accomplish the latter
(command.c only resets query_buf). \e should probably provoke a
recomputation of paren_level after the edit occurs. Offhand I do not
think that any other backslash commands should reset either the buffer
or the counter. Peter, your thoughts?
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > OK, so what do we want to do? Clearing the buffer on a any backslash > > command is clearly not what we want to do. Should we clear the buffer > > on a backslash command _only_ if the number of paren's is not even? If > > we don't clear the counter on a backslash command with uneven parens, do > > we risk trapping people in psql? > > "Trap"? AFAIK \q works in any case. > > \r should reset both the buffer and the counter, and seems to do so, > though I'm not quite seeing where it manages to accomplish the latter > (command.c only resets query_buf). \e should probably provoke a See mainloop.c, line 450. Any backshash command resets the counter. > recomputation of paren_level after the edit occurs. Offhand I do not > think that any other backslash commands should reset either the buffer > or the counter. Peter, your thoughts? Re-doing it for the editor is interesting. The other items like quotes and stuff should also probably be recomputed too. -- Bruce Momjian | http://candle.pha.pa.us pgman@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 <pgman@candle.pha.pa.us> writes: > > OK, so what do we want to do? Clearing the buffer on a any backslash > > command is clearly not what we want to do. Should we clear the buffer > > on a backslash command _only_ if the number of paren's is not even? If > > we don't clear the counter on a backslash command with uneven parens, do > > we risk trapping people in psql? > > "Trap"? AFAIK \q works in any case. > > \r should reset both the buffer and the counter, and seems to do so, > though I'm not quite seeing where it manages to accomplish the latter > (command.c only resets query_buf). \e should probably provoke a > recomputation of paren_level after the edit occurs. Offhand I do not > think that any other backslash commands should reset either the buffer > or the counter. Peter, your thoughts? OK, here is a patch for 7.3. It clears the paren counter only when the buffer is cleared. Forget what I said about recomputing quotes. You can't use backslash commands while you are in quotes. -- Bruce Momjian | http://candle.pha.pa.us pgman@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: src/bin/psql/mainloop.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.43 diff -c -r1.43 mainloop.c *** src/bin/psql/mainloop.c 2001/11/05 17:46:31 1.43 --- src/bin/psql/mainloop.c 2001/12/28 04:41:48 *************** *** 447,453 **** { const char *end_of_cmd = NULL; - paren_level = 0; line[i - prevlen] = '\0'; /* overwrites backslash */ /* is there anything else on the line for the command? */ --- 447,452 ---- *************** *** 469,474 **** --- 468,476 ---- &end_of_cmd); success = slashCmdStatus != CMD_ERROR; + + if (query_buf->len == 0) + paren_level = 0; if ((slashCmdStatus == CMD_SEND || slashCmdStatus == CMD_NEWEDIT) && query_buf->len == 0)
> > --- Bruce Momjian <pgman@candle.pha.pa.us> wrote: > > [...] > > See mainloop.c, line 450. Any backshash command resets the counter. > > > > > recomputation of paren_level after the edit occurs. Offhand I do not > > > think that any other backslash commands should reset either the buffer > > > or the counter. Peter, your thoughts? > > > > Re-doing it for the editor is interesting. The other items like quotes > > and stuff should also probably be recomputed too. > > Whooof! Looks like psql's sitting on a quite big can of worms. > You don't plan to take them all out before 7.2, do you? No, this is all 7.3 discussion. -- Bruce Momjian | http://candle.pha.pa.us pgman@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 writes: > OK, here is a patch for 7.3. It clears the paren counter only when the > buffer is cleared. Forget what I said about recomputing quotes. You > can't use backslash commands while you are in quotes. I don't think this works when the command is \g because you're testing for the cleared buffer too early. Look at what happens under "if (slashCmdStatus == CMD_SEND)". The test should be after that (around line 489 in original copy). -- Peter Eisentraut peter_e@gmx.net
> Bruce Momjian writes:
>
> > OK, here is a patch for 7.3. It clears the paren counter only when the
> > buffer is cleared. Forget what I said about recomputing quotes. You
> > can't use backslash commands while you are in quotes.
>
> I don't think this works when the command is \g because you're testing for
> the cleared buffer too early. Look at what happens under "if
> (slashCmdStatus == CMD_SEND)". The test should be after that (around line
> 489 in original copy).
Are you sure?
test=> (select
test(> \g
ERROR: parser: parse error at or near ""
test(>
I now see the next \p kills me:
test(> \p
( select
test=>
Oops, line 489 doesn't work either:
test=> (select
test(> \g
ERROR: parser: parse error at or near ""
test=>
I now remember how confusing the previous_buffer handling is. It is
this line that is tricky:
/* handle backslash command */
slashCmdStatus = HandleSlashCmds(&line[i],
query_buf->len > 0 ? query_buf : previous_buf,
^^^^^^^^^^^^^^^^^^^^^^^^
&end_of_cmd);
It works now:
test=> (select
test(> \g
ERROR: parser: parse error at or near ""
test(> \p
(select
Patch attached.
--
Bruce Momjian | http://candle.pha.pa.us
pgman@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: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.45
diff -c -r1.45 mainloop.c
*** src/bin/psql/mainloop.c 2001/12/28 05:01:05 1.45
--- src/bin/psql/mainloop.c 2001/12/28 19:12:37
***************
*** 447,453 ****
{
const char *end_of_cmd = NULL;
- paren_level = 0;
line[i - prevlen] = '\0'; /* overwrites backslash */
/* is there anything else on the line for the command? */
--- 447,452 ----
***************
*** 473,479 ****
if ((slashCmdStatus == CMD_SEND || slashCmdStatus == CMD_NEWEDIT) &&
query_buf->len == 0)
{
! /* copy previous buffer to current for for handling */
appendPQExpBufferStr(query_buf, previous_buf->data);
}
--- 472,478 ----
if ((slashCmdStatus == CMD_SEND || slashCmdStatus == CMD_NEWEDIT) &&
query_buf->len == 0)
{
! /* copy previous buffer to current for handling */
appendPQExpBufferStr(query_buf, previous_buf->data);
}
***************
*** 486,491 ****
--- 485,493 ----
appendPQExpBufferStr(previous_buf, query_buf->data);
resetPQExpBuffer(query_buf);
}
+
+ if (query_buf->len == 0 && previous_buf->len == 0)
+ paren_level = 0;
/* process anything left after the backslash command */
i += end_of_cmd - &line[i];
--- Bruce Momjian <pgman@candle.pha.pa.us> wrote: [...] > See mainloop.c, line 450. Any backshash command resets the counter. > > > recomputation of paren_level after the edit occurs. Offhand I do not > > think that any other backslash commands should reset either the buffer > > or the counter. Peter, your thoughts? > > Re-doing it for the editor is interesting. The other items like quotes > and stuff should also probably be recomputed too. Whooof! Looks like psql's sitting on a quite big can of worms. You don't plan to take them all out before 7.2, do you? ===== -- Serguei A. Mokhov <mailto:mokhov@cs.concordia.ca> __________________________________________________ Do You Yahoo!? Send your FREE holiday greetings online! http://greetings.yahoo.com