pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Hamid Akhtar <hamid.akhtar@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Daniel Gustafsson <daniel@yesql.se>
Subject: Re: BUG #16171: Potential malformed JSON in explain output
Date: Fri, 24 Jan 2020 09:28:50 +0000
Message-ID: <157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <495433FC-A268-4956-9B61-5FB135EE5B73@yesql.se>
References: <16171-b72259ab75505fa2@postgresql.org>
	<495433FC-A268-4956-9B61-5FB135EE5B73@yesql.se>

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


view thread (11+ messages)  latest in thread

Message-ID: <157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org>
Permalink:  ../157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org/
Also on:    postgresql.org/message-id/157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: hamid.akhtar@gmail.com, pgsql-hackers@lists.postgresql.org, daniel@yesql.se
  Subject: Re: BUG #16171: Potential malformed JSON in explain output
  In-Reply-To: <157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox