Re: Bug in error reporting for multi-line JSON

From: Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: Bug in error reporting for multi-line JSON
Date: 2021-01-25 13:08:15
Message-ID: CANbhV-FJk_+XTcnuuxujP2FUi7qwC=sUUyEBcbqYoOWmZhxD4A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Thu, Jan 21, 2021 at 6:08 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com> writes:
> > JSON parsing reports the line number and relevant context info
> > incorrectly when the JSON contains newlines. Current code mostly just
> > says "LINE 1" and is misleading for error correction. There were no
> > tests for this previously.
>
> Couple thoughts:
>
> * I think you are wrong to have removed the line number bump that
> happened when report_json_context advances context_start over a
> newline. The case is likely harder to get to now, but it can still
> happen can't it? If it can't, we should remove that whole stanza.

OK, I'm playing around with this to see what is needed.

> * I'd suggest naming the new JsonLexContext field "pos_last_newline";
> "linefeed" is not usually the word we use for this concept. (Although
> actually, it might work better if you make that point to the char
> *after* the newline, in which case "last_linestart" might be the
> right name.)

Yes, OK

> * I'm not enthused about back-patching. This behavior seems like an
> improvement, but that doesn't mean people will appreciate changing it
> in stable branches.

OK, as you wish.

Thanks for the review, will post again soon with an updated patch.

--
Simon Riggs http://www.EnterpriseDB.com/

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2021-01-26 07:00:00 BUG #16837: Invalid memory access on \h in psql
Previous Message PG Bug reporting form 2021-01-25 08:39:02 BUG #16836: performance drop while query information_schema since switch from postgres 10 to 12

Browse pgsql-hackers by date

  From Date Subject
Next Message 'Alvaro Herrera' 2021-01-25 13:11:10 Re: libpq debug log
Previous Message Justin Pryzby 2021-01-25 12:58:49 Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly