Re: Patch for Improved Syntax Error Reporting

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Dave Page <dpage(at)vale-housing(dot)co(dot)uk>, Neil Padgett <npadgett(at)redhat(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Patch for Improved Syntax Error Reporting
Date: 2001-08-02 22:25:26
Message-ID: 200108022225.f72MPQh26520@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


Yes, that is how I thought they would have to do it. Seems pretty
complicated to me, at least, but if no one complains, it is fine by me.

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> significant new effort from the client app is where it reformats queries
> >> before sending them on to the parser ... but that is knowledge you've
> >> already built into the client, no? You just have to remember what you
> >> did so that you can map a backend error index into the original text.
>
> > That seems particularly hard.
>
> No, it's absolutely and utterly trivial. As you collapse the query to
> what you send, you are copying it from an input buffer to an output
> workspace, no? As you do this, you build an index array that holds the
> input index of each output character. Then when you get an error index
> from the backend, you look in this array to find the source-text index
> you want to point at. Then you do what Neil originally proposed to do
> in the backend (which was what, about a dozen lines of code?) to produce
> an error report, if the style of error report he proposed was what makes
> sense for your client app. Or you do something different if that's what
> makes sense for your app.
>
> ninchars = strlen(inputbuf);
> outbuf = (char *) malloc((ninchars+1) * sizeof(char));
> maparray = (int *) malloc((ninchars+1) * sizeof(int));
>
> outindex = 0;
> for (inindex = 0 ; inindex < ninchars; inindex++)
> {
> if (want to send this char to backend)
> {
> outbuf[outindex] = inputbuf[inindex];
> maparray[outindex] = inindex;
> outindex++;
> }
> }
>
> outbuf[outindex] = inputbuf[inindex]; /* terminating \0 */
> maparray[outindex] = inindex;
>
> This is essentially three more lines than what you had anyway
> (everything except the lines mentioning maparray was there before).
> When you get an error index back from the backend, it takes one
> more line of code to map it back to an input-string index.
> Not exactly rocket science, IMHO.
>
> Four additional lines of code on the client side seems a *very* small
> price to pay for giving client apps the freedom to do the right thing
> for their style of user interface.
>
> regards, tom lane
>
> PS: okay, five lines. I forgot to count free(maparray) ;-)
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

In response to

Browse pgsql-patches by date

  From Date Subject
Next Message Fernando Nasser 2001-08-02 23:06:20 Re: Revised Patch to allow multiple table locks in "Unison"
Previous Message Tom Lane 2001-08-02 22:23:23 Re: Patch for Improved Syntax Error Reporting