Re: jsonpath

On 06.03.2018 00:29, Tomas Vondra wrote:

> Hi,
>
> The patch no longer applies - it got broken by fd1a421fe66 which changed
> columns in pg_proc. A rebase is needed.
>
> Fixing it is pretty simle, so I've done that locally and tried to run
> 'make check' under valgrind. And I got a bunch of reports about
> uninitialised values.

Thank you very much for your discovery. It is my fault that I never ran
PostgreSQL under valgrind. Rebased and fixed patches are attached.

> Full report attached, but in general there seem to
> be two types of failures:
>
> Conditional jump or move depends on uninitialised value(s)
> at 0x57BB47A: vfprintf (in /usr/lib64/libc-2.24.so)
> by 0x57E5478: vsnprintf (in /usr/lib64/libc-2.24.so)
> by 0xA926D3: pvsnprintf (psprintf.c:121)
> by 0x723E03: appendStringInfoVA (stringinfo.c:130)
> by 0x723D58: appendStringInfo (stringinfo.c:87)
> by 0x76BEFF: _outCoerceViaIO (outfuncs.c:1413)
> by 0x776F99: outNode (outfuncs.c:3978)
> by 0x76D7E7: _outJsonCoercion (outfuncs.c:1779)
> by 0x777CB9: outNode (outfuncs.c:4398)
> by 0x76D507: _outJsonExpr (outfuncs.c:1752)
> by 0x777CA1: outNode (outfuncs.c:4395)
> by 0x767000: _outList (outfuncs.c:187)
> by 0x776874: outNode (outfuncs.c:3753)
> by 0x76A4D2: _outTableFunc (outfuncs.c:1068)
> by 0x776D89: outNode (outfuncs.c:3912)
> by 0x7744FD: _outRangeTblEntry (outfuncs.c:3209)
> by 0x777959: outNode (outfuncs.c:4290)
> by 0x767000: _outList (outfuncs.c:187)
> by 0x776874: outNode (outfuncs.c:3753)
> by 0x773713: _outQuery (outfuncs.c:3049)
> Uninitialised value was created by a stack allocation
> at 0x5B0C19: base_yyparse (gram.c:26287)
>
> This happens when _outCoerceViaIO tries to output 'location' field
> (that's line 1413), so I guess it's not set/copied somewhere.

Yes, JSON FORMAT location was not set in gram.y.

> The second failure looks like this:
>
> Conditional jump or move depends on uninitialised value(s)
> at 0x49E58B: ginFillScanEntry (ginscan.c:72)
> by 0x49EB56: ginFillScanKey (ginscan.c:221)
> by 0x49EF72: ginNewScanKey (ginscan.c:369)
> by 0x4A3875: gingetbitmap (ginget.c:1807)
> by 0x4F620B: index_getbitmap (indexam.c:727)
> by 0x6EE342: MultiExecBitmapIndexScan (nodeBitmapIndexscan.c:104)
> by 0x6DA8F8: MultiExecProcNode (execProcnode.c:506)
> by 0x6EC53D: BitmapHeapNext (nodeBitmapHeapscan.c:118)
> by 0x6DC26D: ExecScanFetch (execScan.c:95)
> by 0x6DC308: ExecScan (execScan.c:162)
> by 0x6ED7E5: ExecBitmapHeapScan (nodeBitmapHeapscan.c:730)
> by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
> by 0x6E5961: ExecProcNode (executor.h:239)
> by 0x6E5E25: fetch_input_tuple (nodeAgg.c:406)
> by 0x6E8091: agg_retrieve_direct (nodeAgg.c:1736)
> by 0x6E7C84: ExecAgg (nodeAgg.c:1551)
> by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
> by 0x6D1361: ExecProcNode (executor.h:239)
> by 0x6D3BB7: ExecutePlan (execMain.c:1721)
> by 0x6D1917: standard_ExecutorRun (execMain.c:361)
> Uninitialised value was created by a heap allocation
> at 0xA64FDC: palloc (mcxt.c:858)
> by 0x938636: gin_extract_jsonpath_query (jsonb_gin.c:630)
> by 0x938AB6: gin_extract_jsonb_query (jsonb_gin.c:746)
> by 0xA340C0: FunctionCall7Coll (fmgr.c:1201)
> by 0x49EE7F: ginNewScanKey (ginscan.c:313)
> by 0x4A3875: gingetbitmap (ginget.c:1807)
> by 0x4F620B: index_getbitmap (indexam.c:727)
> by 0x6EE342: MultiExecBitmapIndexScan (nodeBitmapIndexscan.c:104)
> by 0x6DA8F8: MultiExecProcNode (execProcnode.c:506)
> by 0x6EC53D: BitmapHeapNext (nodeBitmapHeapscan.c:118)
> by 0x6DC26D: ExecScanFetch (execScan.c:95)
> by 0x6DC308: ExecScan (execScan.c:162)
> by 0x6ED7E5: ExecBitmapHeapScan (nodeBitmapHeapscan.c:730)
> by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
> by 0x6E5961: ExecProcNode (executor.h:239)
> by 0x6E5E25: fetch_input_tuple (nodeAgg.c:406)
> by 0x6E8091: agg_retrieve_direct (nodeAgg.c:1736)
> by 0x6E7C84: ExecAgg (nodeAgg.c:1551)
> by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
> by 0x6D1361: ExecProcNode (executor.h:239)
>
> So the extra_data allocated in gin_extract_jsonpath_query() get to
> ginFillScanEntry() uninitialised.

Yes, only the first element of extra_data[] was initialized and used later in
gin_consistent_jsonb(), but ginFillScanEntry() wants all of them to be
initialized.

> Both seem like a valid issues, I think.
>
> regards

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company