From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
cc: Hamid Akhtar <hamid.akhtar@gmail.com>, pgsql-hackers@lists.postgresql.org
Subject: Re: BUG #16171: Potential malformed JSON in explain output
In-reply-to: <5217695A-8776-4F59-823A-B391DCA1F601@yesql.se>
References: <16171-b72259ab75505fa2@postgresql.org>
 <495433FC-A268-4956-9B61-5FB135EE5B73@yesql.se>
 <157985813081.742.12901374861257824963.pgcf@coridan.postgresql.org>
 <9342.1580585875@sss.pgh.pa.us>
 <5217695A-8776-4F59-823A-B391DCA1F601@yesql.se>
Comments: In-reply-to Daniel Gustafsson <daniel@yesql.se>
	message dated "Sun, 02 Feb 2020 13:08:07 +0100"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <30660.1580745275.0@sss.pgh.pa.us>
Date: Mon, 03 Feb 2020 10:56:09 -0500
Message-ID: <30719.1580745369@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <30660.1580745275.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable

Daniel Gustafsson <daniel@yesql.se> writes:
> On 1 Feb 2020, at 20:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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 whee=
l
> reinventing?

It occurred to me that as long as this is an uncommittable hack anyway,
we could feed the EXPLAIN data to jsonb_in and then hot-wire the jsonb
code to whine about duplicate keys.  So attached, for the archives' sake,
is an improved version that does that.  I still don't find any problems
(other than the one we're fixing here); though no doubt if I reverted
100136849 it'd complain about that.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="0002-check-json-validity-2.patch";
	charset="us-ascii"
Content-ID: <30660.1580745275.2@sss.pgh.pa.us>
Content-Description: 0002-check-json-validity-2.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/au=
to_explain.c
index f69dde8..1945a77 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -18,6 +18,7 @@
 #include "commands/explain.h"
 #include "executor/instrument.h"
 #include "jit/jit.h"
+#include "utils/builtins.h"
 #include "utils/guc.h"
 =

 PG_MODULE_MAGIC;
@@ -397,6 +398,10 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
 			{
 				es->str->data[0] =3D '{';
 				es->str->data[es->str->len - 1] =3D '}';
+
+				/* Verify that it's valid JSON by feeding to jsonb_in */
+				(void) DirectFunctionCall1(jsonb_in,
+										   CStringGetDatum(es->str->data));
 			}
 =

 			/*
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/js=
onb_util.c
index edec657..b9038fa 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -1915,6 +1915,9 @@ uniqueifyJsonbObject(JsonbValue *object)
 				if (ptr !=3D res)
 					memcpy(res, ptr, sizeof(JsonbPair));
 			}
+			else
+				elog(WARNING, "dropping duplicate jsonb key %s",
+					 ptr->key.val.string.val);
 			ptr++;
 		}
 =

diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expect=
ed/jsonb.out
index a70cd0b..2db14e1 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -3488,6 +3488,9 @@ SELECT '{"ff":{"a":12,"b":16},"qq":123}'::jsonb;
 (1 row)
 =

 SELECT '{"aa":["a","aaa"],"qq":{"a":12,"b":16,"c":["c1","c2"],"d":{"d1":"=
d1","d2":"d2","d1":"d3"}}}'::jsonb;
+WARNING:  dropping duplicate jsonb key d1
+LINE 1: SELECT '{"aa":["a","aaa"],"qq":{"a":12,"b":16,"c":["c1","c2"...
+               ^
                                               jsonb                      =
                         =

 -------------------------------------------------------------------------=
-------------------------
  {"aa": ["a", "aaa"], "qq": {"a": 12, "b": 16, "c": ["c1", "c2"], "d": {"=
d1": "d3", "d2": "d2"}}}
@@ -4021,6 +4024,8 @@ select jsonb_concat('{"d": "test", "a": [1, 2]}', '{=
"g": "test2", "c": {"c1":1,
 (1 row)
 =

 select '{"aa":1 , "b":2, "cq":3}'::jsonb || '{"cq":"l", "b":"g", "fg":fal=
se}';
+WARNING:  dropping duplicate jsonb key baacq
+WARNING:  dropping duplicate jsonb key cq
                   ?column?                   =

 ---------------------------------------------
  {"b": "g", "aa": 1, "cq": "l", "fg": false}
@@ -4033,6 +4038,7 @@ select '{"aa":1 , "b":2, "cq":3}'::jsonb || '{"aq":"=
l"}';
 (1 row)
 =

 select '{"aa":1 , "b":2, "cq":3}'::jsonb || '{"aa":"l"}';
+WARNING:  dropping duplicate jsonb key aacq
            ?column?           =

 ------------------------------
  {"b": 2, "aa": "l", "cq": 3}

------- =_aaaaaaaaaa0--