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]]
connect to 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 distribution terms
\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 aggregate functions
\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
\echo TEXT write text to standard output
\encoding ENCODING set client encoding
\f STRING set field separator
\g FILENAME send SQL command to server (and write results to file or |pipe)
\h NAME help on syntax of SQL commands, * for all commands
\H toggle HTML output mode (currently off)
\i FILENAME execute commands from file
\l list all databases
\lo_export, \lo_import, \lo_list, \lo_unlink
large object operations
\o FILENAME send all query results to file or |pipe
\p show the content of the current query buffer
\pset VAR 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 output stream (see \o)
\r reset (clear) the query buffer
\s FILENAME print history or save it to file
\set NAME VALUE set internal variable
\t show only rows (currently off)
\T TEXT set HTML table tag attributes
\unset NAME unset (delete) internal variable
\w FILENAME write current query buffer to file
\x toggle expanded output (currently off)
\z list table access privileges
\! [COMMAND] execute command in shell or start 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, Pennsylvania 19026Bruce 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.
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, Pennsylvania 19026Bruce 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, Pennsylvania 19026Bruce 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, Pennsylvania 19026Bruce 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 19026Attachments:
/bjm/difftext/plainDownload
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?
=====
--
Serguei A. Mokhov <mailto:mokhov@cs.concordia.ca>
__________________________________________________
Do You Yahoo!?
Send your FREE holiday greetings online!
http://greetings.yahoo.com
--- 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, Pennsylvania 19026
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 19026Attachments:
/pgpatches/parentext/plainDownload
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];