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

> On 1 Feb 2020, at 20:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Hamid Akhtar <hamid.akhtar@gmail.com> writes:
>> I've reviewed and verified this patch and IMHO, this is ready to be committed.
> 
> I took a look at this and I don't think it's really going in the right
> direction.  ISTM the clear intent of this code was to attach the "Subplans
> Removed" item as a field of the parent [Merge]Append node, but the author
> forgot about the intermediate "Plans" array.  So I think that, rather than
> doubling down on a mistake, we ought to move where the field is generated
> so that it *is* a field of the parent node.

Right, that makes sense; +1 on the attached 0001 patch.

> 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.

> 0002 attached isn't committable, because nobody would want the overhead
> in production, but it seems like a good trick to keep up our sleeves.

Thats a neat trick, I wonder if it would be worth maintaining a curated list of
these tricks in a README under src/test to help others avoid/reduce wheel
reinventing?

cheers ./daniel