Re: psql patch

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: "Jeroen T(dot) Vermeulen" <jtv(at)xs4all(dot)nl>
Cc: PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Neil Conway <neilc(at)samurai(dot)com>
Subject: Re: psql patch
Date: 2003-03-21 03:40:03
Message-ID: 200303210340.h2L3e3u15742@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


OK, I applied your suggested cleanup. I don't think I can move the code
up or I would not be able to do nested /* */ comments, which the code
seems to support.

---------------------------------------------------------------------------

Jeroen T. Vermeulen wrote:
> On Thu, Mar 20, 2003 at 05:13:05PM -0500, Bruce Momjian wrote:
> >
> > I have applied the following patch to fix a bug in the new psql patch.
> > It wasn't handling multi-line /* */ comments properly. Jeroen, would
> > you review the change. The rest of the changes look very good.
>
> Yes, come to think of it I can see now that it also wouldn't deal
> with special sequences like '--' or unmatched quotes and parentheses
> very well either. Nor did the original version, from the looks of
> it, so I'd like to suggest moving the whole "if (in_xcomment)" clause
> up a little in the else-if chain, to just below the "if (in_quote)"
> one. That would make the thing ignore all special sequences except
> '*/' when inside an xcomment, and all sequences inside a quote
> (except a closing quote, of course). Plus, just in case any
> legitimate expression could give rise to the sequence "*/" ouside
> of an xcomment, that would also be handled properly.
>
> BTW your version could be simplified a little:
>
> /* [ "if (in_quote)" clause here ] */
>
> /* end of extended comment? */
> else if (in_xcomment)
> {
> if ((line[i] == '*') &&
> (line[i + thislen] == '/') &&
> !--in_xcomment)
> ADVANCE_1;
> }
>
> /* start of extended comment? */
> /* [ '/*' clause here ] */
>
>
> Lessee... I hope this says it all, but if you like I can submit
> a patch to today's CVS version.
>
>
> Jeroen
>
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Attachment Content-Type Size
unknown_filename text/plain 883 bytes

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2003-03-21 03:55:55 Re: Sequence patch has problems
Previous Message Tom Lane 2003-03-21 02:10:54 Sequence patch has problems