Thread: psql and comments
While you're at it...
The following example shows psql correctly clearing its input buffer
when a line containing *only* a comment is seen, but not completely
clearing the buffer (or not realizing that it is cleared; note the
changed prompt) if the comment is at the end of a valid query.
postgres=> -- comment
postgres=> select 'hi'; -- comment
?column?
--------
hi
(1 row)
postgres->
- Thomas
--
Thomas Lockhart lockhart@alumni.caltech.edu
South Pasadena, California
> While you're at it... > > The following example shows psql correctly clearing its input buffer > when a line containing *only* a comment is seen, but not completely > clearing the buffer (or not realizing that it is cleared; note the > changed prompt) if the comment is at the end of a valid query. > > postgres=> -- comment > postgres=> select 'hi'; -- comment > ?column? > -------- > hi > (1 row) > > postgres-> But aren't they _in_ a new statement, that begins with '--'? -- 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
On Oct 6, Thomas Lockhart mentioned: > The following example shows psql correctly clearing its input buffer > when a line containing *only* a comment is seen, but not completely > clearing the buffer (or not realizing that it is cleared; note the > changed prompt) if the comment is at the end of a valid query. > > postgres=> -- comment > postgres=> select 'hi'; -- comment > ?column? > -------- > hi > (1 row) > > postgres-> That has been noted by me as well. From looking at the code I see that someone intended to do something quite different in this case, like print the comment on top of the query being echoed, I think. But I couldn't really follow that. Anyway, I'm going to end up rewriting that parser anyway, so that will be taken care of. I was almost about to use flex but the Windows crowd probably wouldn't find that too funny. (The Windows crowd won't find this thing funny anyway, since I have no clue what #ifdef's I need for that. Someone else will have to do a looong compile&fix session.) The question I have though is, is there a reason, besides efficiency, that psql doesn't just send the comment to the backend with the query? The backend does accept comments last time I checked. Perhaps someone will one day write something that makes some use of those comments on the backend (thus conflicting with the very definition of "comment", but maybe a logger) and it would remove some load out of psql. -- Peter Eisentraut - peter_e@gmx.net http://yi.org/peter-e/
> Anyway, I'm going to end up rewriting that parser anyway, so that will be > taken care of. I was almost about to use flex but the Windows crowd > probably wouldn't find that too funny. (The Windows crowd won't find this > thing funny anyway, since I have no clue what #ifdef's I need for that. > Someone else will have to do a looong compile&fix session.) > > The question I have though is, is there a reason, besides efficiency, that > psql doesn't just send the comment to the backend with the query? The > backend does accept comments last time I checked. Perhaps someone will one > day write something that makes some use of those comments on the backend > (thus conflicting with the very definition of "comment", but maybe a > logger) and it would remove some load out of psql. Remove it. Send it to the backend. -- 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
> > The following example shows psql correctly clearing its input buffer
> > when a line containing *only* a comment is seen, but not completely
> > clearing the buffer (or not realizing that it is cleared; note the
> > changed prompt) if the comment is at the end of a valid query.
> >
> > postgres=> -- comment
> > postgres=> select 'hi'; -- comment
> > ?column?
> > --------
> > hi
> > (1 row)
> >
> > postgres->
> But aren't they _in_ a new statement, that begins with '--'?
?? Sure, that's what psql thinks. But the first case shown above
should also begin a new statement, changing the prompt (it doesn't,
because after stripping the comment there are zero blanks in the
line). I don't think that is the right behavior though.
Things aren't a big problem the way they stand, but istm that a
completely blank line (after stripping single-line comments) may as
well be the same as an empty line,and that psql could figure that out.
- Thomas
--
Thomas Lockhart lockhart@alumni.caltech.edu
South Pasadena, California
> The question I have though is, is there a reason, besides efficiency, that
> psql doesn't just send the comment to the backend with the query? The
> backend does accept comments last time I checked. Perhaps someone will one
> day write something that makes some use of those comments on the backend
> (thus conflicting with the very definition of "comment", but maybe a
> logger) and it would remove some load out of psql.
Efficiency is all, along with (probably) the backend being unhappy
getting *only* a comment and no query.
- Thomas
--
Thomas Lockhart lockhart@alumni.caltech.edu
South Pasadena, California
> > > The following example shows psql correctly clearing its input buffer > > > when a line containing *only* a comment is seen, but not completely > > > clearing the buffer (or not realizing that it is cleared; note the > > > changed prompt) if the comment is at the end of a valid query. > > > > > > postgres=> -- comment > > > postgres=> select 'hi'; -- comment > > > ?column? > > > -------- > > > hi > > > (1 row) > > > > > > postgres-> > > But aren't they _in_ a new statement, that begins with '--'? > > ?? Sure, that's what psql thinks. But the first case shown above > should also begin a new statement, changing the prompt (it doesn't, > because after stripping the comment there are zero blanks in the > line). I don't think that is the right behavior though. > > Things aren't a big problem the way they stand, but istm that a > completely blank line (after stripping single-line comments) may as > well be the same as an empty line,and that psql could figure that out. I see your point in the above example. I will wait for the psql/libpq cleaner-upper to finish, and take a look at it. -- 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
> > The question I have though is, is there a reason, besides efficiency, that > > psql doesn't just send the comment to the backend with the query? The > > backend does accept comments last time I checked. Perhaps someone will one > > day write something that makes some use of those comments on the backend > > (thus conflicting with the very definition of "comment", but maybe a > > logger) and it would remove some load out of psql. > > Efficiency is all, along with (probably) the backend being unhappy > getting *only* a comment and no query. > That is fixed now. External interfaces showed problems, as the perl MySQL test showed. -- 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
>> Things aren't a big problem the way they stand, but istm that a
>> completely blank line (after stripping single-line comments) may as
>> well be the same as an empty line,and that psql could figure that out.
There was talk earlier of changing the behavior so that psql would
forward comments to the backend, rather than stripping them. One
potential annoyance if we do that is that (I think) all the regress
test expected outputs will change because comments will then appear
in them.
I'd be inclined to maintain the current behavior. psql has to have a
simple parser in it anyway to know when it has a complete query it can
send to the backend --- so it must know what is a comment and what is
not. Removing the comments is not really going to add much complexity
AFAICS.
regards, tom lane
On Oct 6, Bruce Momjian mentioned: > > postgres=> select 'hi'; -- comment > But aren't they _in_ a new statement, that begins with '--'? Good point. But it's still kind of counterintuitive, isn't it? Especially since something that begins with a '--' can't ever become a useful statement. The problem seems to be in the parsing stages: the query is send off before the comment is encountered. The alternative solution of putting query and comment on the same line would be => select 'hi' -- comment ; but that doesn't work at all obviously. Meanwhile it might be worth pondering if => select 'hi' -- comment \g should be allowed, since => select 'hi' \g -- comment does something different. (And try removing that file if you're an unexperienced user.) Regarding that last line, I just discovered a possible incompatibility between the official and my current version. The official version creates a file "-- comment" whereas mine makes a file "--" because I now have word splitting and quoting rules and the like (so \g '-- comment' would work). Something to else ponder. -- Peter Eisentraut - peter_e@gmx.net http://yi.org/peter-e/
Bruce Momjian <maillist@candle.pha.pa.us> writes:
>> Efficiency is all, along with (probably) the backend being unhappy
>> getting *only* a comment and no query.
> That is fixed now.
Is it? postgres.c treats an all-whitespace input as an empty query,
but if you pass it a comment and nothing else it will cycle the parser/
planner/executor, and I'm not sure every phase of that process behaves
reasonably on empty input. Also, that path will not produce the
"empty query" response code that you get from all-whitespace input.
I *think* libpq doesn't depend on that anymore, but other frontend
libraries might...
regards, tom lane
> Bruce Momjian <maillist@candle.pha.pa.us> writes: > >> Efficiency is all, along with (probably) the backend being unhappy > >> getting *only* a comment and no query. > > > That is fixed now. > > Is it? postgres.c treats an all-whitespace input as an empty query, > but if you pass it a comment and nothing else it will cycle the parser/ > planner/executor, and I'm not sure every phase of that process behaves > reasonably on empty input. Also, that path will not produce the > "empty query" response code that you get from all-whitespace input. > I *think* libpq doesn't depend on that anymore, but other frontend > libraries might... postgres -D /u/pg/data testPOSTGRES backend interactive interface $Revision: 1.130 $ $Date: 1999/09/29 16:06:10 $backend>-- testbackend> Is that what you mean? -- 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 <maillist@candle.pha.pa.us> writes:
>>>> That is fixed now.
>>
>> Is it? postgres.c treats an all-whitespace input as an empty query,
>> but if you pass it a comment and nothing else it will cycle the parser/
>> planner/executor, and I'm not sure every phase of that process behaves
>> reasonably on empty input. Also, that path will not produce the
>> "empty query" response code that you get from all-whitespace input.
>> I *think* libpq doesn't depend on that anymore, but other frontend
>> libraries might...
> postgres -D /u/pg/data test
> POSTGRES backend interactive interface
> $Revision: 1.130 $ $Date: 1999/09/29 16:06:10 $
backend> -- test
backend>
> Is that what you mean?
OK, so the parser/planner/executor can cope with dummy input. That's
good. There's still the problem of returning an 'empty query' response
to the frontend. I think you'd probably need to hack up postgres.c
so that when the querytree list produced by the parser is NIL, the
IsEmptyQuery flag gets set --- this could be done instead of, rather
than in addition to, the current test for an all-whitespace input
buffer.
regards, tom lane
> OK, so the parser/planner/executor can cope with dummy input. That's > good. There's still the problem of returning an 'empty query' response > to the frontend. I think you'd probably need to hack up postgres.c > so that when the querytree list produced by the parser is NIL, the > IsEmptyQuery flag gets set --- this could be done instead of, rather > than in addition to, the current test for an all-whitespace input > buffer. The system may already return that. My gram.y code tests for empty queries, and should be doing the right thing. Not sure, because I am not sure what to check for. -- 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
On Oct 7, Bruce Momjian mentioned: > > Things aren't a big problem the way they stand, but istm that a > > completely blank line (after stripping single-line comments) may as > > well be the same as an empty line,and that psql could figure that out. > > I see your point in the above example. I will wait for the psql/libpq > cleaner-upper to finish, and take a look at it. Oh, now I'm cleaning up libpq as well??? 8-} Well anyway, by a vote of 1 1/2 to 1 psql will strip all comments before sending a query, probably in a C pre-processor kind of way. -- Peter Eisentraut Sernanders vaeg 10:115 peter_e@gmx.net 75262 Uppsala http://yi.org/peter-e/ Sweden
On Oct 7, Tom Lane mentioned: > There was talk earlier of changing the behavior so that psql would > forward comments to the backend, rather than stripping them. One > potential annoyance if we do that is that (I think) all the regress > test expected outputs will change because comments will then appear > in them. That is a somewhat separate issue, but good that you bring it up. In my cleaning ways I noticed that the -e vs. -E switches weren't applied correctly so I set that straight to an extent. The regression tests rely on -e to echo the query back correctly, not the one actually sent to the backend, so that could be tweaked. Luckily, the regression tests don't make extensive use of the backslash commands, the issue being that their output might change. I only found three backslash commands in the whole regression tests. One occurence does something like this: some query; *** comment *** comment \p \r more queries; which should probably be changed anyway to something like -- comment -- comment The other case is CREATE TEMP TABLE temptest(col int); -- test temp table deletion \c regression SELECT * FROM temptest; which still works as I just confirmed, and the output of \c gets eaten in -q mode anyway. Seems it's still safe. -- Peter Eisentraut Sernanders vaeg 10:115 peter_e@gmx.net 75262 Uppsala http://yi.org/peter-e/ Sweden
> On Oct 7, Bruce Momjian mentioned: > > > > Things aren't a big problem the way they stand, but istm that a > > > completely blank line (after stripping single-line comments) may as > > > well be the same as an empty line,and that psql could figure that out. > > > > I see your point in the above example. I will wait for the psql/libpq > > cleaner-upper to finish, and take a look at it. > > Oh, now I'm cleaning up libpq as well??? 8-} > > Well anyway, by a vote of 1 1/2 to 1 psql will strip all comments before > sending a query, probably in a C pre-processor kind of way. Looks like we are going for a 7.0 next, so feel free to remove very old functions. -- 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
> Luckily, the regression tests don't make extensive use of the backslash
> commands, the issue being that their output might change. I only found
> three backslash commands in the whole regression tests. One occurence does
> something like this:
> some query;
> *** comment
> *** comment
> \p
> \r
> more queries;
> which should probably be changed anyway to something like
> -- comment
> -- comment
Actually, this is probably testing that the buffer reset actually
clears the lines, which wouldn't be as obvious if there were only
legal SQL preceeding it. Maybe leave it as-is??
- Thomas
--
Thomas Lockhart lockhart@alumni.caltech.edu
South Pasadena, California
On Oct 11, Thomas Lockhart mentioned: > > some query; > > *** comment > > *** comment > > \p > > \r > > more queries; > > which should probably be changed anyway to something like > > -- comment > > -- comment > > Actually, this is probably testing that the buffer reset actually > clears the lines, which wouldn't be as obvious if there were only > legal SQL preceeding it. Maybe leave it as-is?? I think I figured that out: the *** comments actually show up in the output of the regression tests as an aid to a person glancing at the results. If the regression tests wanted to test psql then they could certainly do a lot more fun things than that. I was planning on implementing an \echo command which could easily be dropped in there as a more elegant solution. Which makes me think. The server regression tests should certainly not rely on some particular psql functionality, just as a matter of principle. But if I'm ever really bored I could write a separate psql regression test. No promise though. -Peter -- Peter Eisentraut Sernanders vaeg 10:115 peter_e@gmx.net 75262 Uppsala http://yi.org/peter-e/ Sweden
This is fixed in the current source tree. > > > The following example shows psql correctly clearing its input buffer > > > when a line containing *only* a comment is seen, but not completely > > > clearing the buffer (or not realizing that it is cleared; note the > > > changed prompt) if the comment is at the end of a valid query. > > > > > > postgres=> -- comment > > > postgres=> select 'hi'; -- comment > > > ?column? > > > -------- > > > hi > > > (1 row) > > > > > > postgres-> > > But aren't they _in_ a new statement, that begins with '--'? > > ?? Sure, that's what psql thinks. But the first case shown above > should also begin a new statement, changing the prompt (it doesn't, > because after stripping the comment there are zero blanks in the > line). I don't think that is the right behavior though. > > Things aren't a big problem the way they stand, but istm that a > completely blank line (after stripping single-line comments) may as > well be the same as an empty line,and that psql could figure that out. > > - Thomas > > -- > Thomas Lockhart lockhart@alumni.caltech.edu > South Pasadena, California > -- 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