Re: BUG #16171: Potential malformed JSON in explain output

[ cc'ing depesz to see what he thinks about this ]

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
> On 1 Feb 2020, at 20:37, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> This does lead to some field
>> order rearrangement in text mode, as per the regression test changes,
>> but I think that's not a big deal. (A change can only happen if there
>> are initplan(s) attached to the parent node.)

> Does that prevent backpatching this, or are we Ok with EXPLAIN text output not
> being stable across minors? AFAICT Pg::Explain still works fine with this
> change, but mileage may vary for other parsers.

I'm not sure about that either. It should be a clear win for parsers
of the non-text formats, because now we're generating valid
JSON-or-whatever where we were not before. But it's not too hard to
imagine that someone's ad-hoc parser of text output would break,
depending on how much it relies on field order rather than indentation
to make sense of things.

In the background of all this is that cases where it matters must be
pretty thin on the ground so far, else we'd have gotten complaints
sooner. So we shouldn't really assume that everyone's parser handles
such cases at all yet.

I'm a little bit inclined to back-patch, on the grounds that JSON
output is hopelessly broken without this, and any text-mode parsers
that need work would need work by September anyway. But I could
easily be argued into not back-patching.

Another approach we could consider is putting your patch in the
back branches and mine in HEAD. I'm not sure that's a good idea;
it trades short-term stability of the text format for a long-term
mess in the non-text formats. But maybe somebody will argue for it.

Thoughts?

regards, tom lane