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

From: Hamid Akhtar <hamid(dot)akhtar(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: Re: BUG #16171: Potential malformed JSON in explain output
Date: 2020-01-24 09:28:50
Message-ID: 157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed

I've reviewed and verified this patch and IMHO, this is ready to be committed.

So I have verified this patch against the tip of REL_12_STABLE branch (commit c4c76d19).
- git apply <patch> works without issues.

Although the patch description mentions that it fixes a malformed JSON in explain output by creating a "Plan" group, this also fixes the same malformation issue for XML and YAML formats. It does not impact the text output though.

I'm sharing the problematic part of output for an unpatched version for JSON, XML, and YAML formats:
-- JSON
"Plans": [
"Subplans Removed": 1,
{
"Node Type": "Seq Scan",

-- XML
<Plans>
<Subplans-Removed>1</Subplans-Removed>
<Plan>

-- YAML
Plans:
Subplans Removed: 1
- Node Type: "Seq Scan"

The patched version gives the following and correct output:
-- JSON
"Plans": [
{
"Subplans Removed": 1
},
{
"Node Type": "Seq Scan",

-- XML
<Plans>
<Plan>
<Subplans-Removed>1</Subplans-Removed>
</Plan>
<Plan>

-- YAML
Plans:
- Subplans Removed: 1
- Node Type: "Seq Scan"

Following is the query that I used for validating the output. I picked it up (and simplified) from "src/test/regress/sql/partition_prune.sql". You can probably come up with a simpler query, but this does the job. The query below gives the output in JSON format:
----
create table ab (a int not null, b int not null) partition by list (a);
create table ab_a2 partition of ab for values in(2) partition by list (b);
create table ab_a2_b1 partition of ab_a2 for values in (1);
create table ab_a1 partition of ab for values in(1) partition by list (b);
create table ab_a1_b1 partition of ab_a1 for values in (2);

-- Disallow index only scans as concurrent transactions may stop visibility
-- bits being set causing "Heap Fetches" to be unstable in the EXPLAIN ANALYZE
-- output.
set enable_indexonlyscan = off;

prepare ab_q1 (int, int, int) as
select * from ab where a between $1 and $2 and b <= $3;

-- Execute query 5 times to allow choose_custom_plan
-- to start considering a generic plan.
execute ab_q1 (1, 8, 3);
execute ab_q1 (1, 8, 3);
execute ab_q1 (1, 8, 3);
execute ab_q1 (1, 8, 3);
execute ab_q1 (1, 8, 3);

explain (format json, analyze, costs off, summary off, timing off) execute ab_q1 (2, 2, 3);

deallocate ab_q1;
drop table ab;
----

The new status of this patch is: Ready for Committer

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Daniel Gustafsson 2020-01-24 13:04:57 Re: BUG #16227: Loss database tables automatically in a couple of days
Previous Message Magnus Hagander 2020-01-24 09:05:20 Re: BUG #16225: EL8 - PGDG postgresql* packages conflict with appstream postgresql packages

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Moench-Tegeder 2020-01-24 09:29:37 Re: New feature proposal (trigger)
Previous Message Pavel Stehule 2020-01-24 09:14:07 Re: New feature proposal (trigger)