pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
8+ messages / 4 participants
[nested] [flat]

Commits from this thread:
  2017-10-27 7e5e8b36d4 acd3287e43 66104119e1 9d15b8b36a 80e79718d0 ddde3b4f3e d5b760ecb5  Fix crash when columns have been added to the end of a view.
* BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
@ 2017-10-26 18:40 samuel.horwitz@gmail.com
  2017-10-26 19:09 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Dmitry Dolgov <9erthalion6@gmail.com>
  2017-10-26 19:24 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 2 replies; 8+ messages in thread

From: samuel.horwitz@gmail.com @ 2017-10-26 18:40 UTC (permalink / raw)
  To: pgsql-bugs; +Cc: samuel.horwitz@gmail.com

The following bug has been logged on the website:

Bug reference:      14876
Logged by:          Samuel Horwitz
Email address:      samuel.horwitz@gmail.com
PostgreSQL version: 10.0
Operating system:   OS X (but running inside Docker official image)
Description:        

I have a table that includes a JSONB column. I have a view that is based on
this table that reflects this JSONB column. I have yet another view, based
on the first view, that passes rows from the first view into a stored
PL/PGSQL procedure as anyelement. This stored procedure builds a JSON object
from it's input and returns that new JSON object. One of the values on the
new JSON object is the value of the JSONB column that has gone through two
views by this point, into the procedure.

At this point everything works. Selecting from the second view is fine.

However, if I alter the stored procedure's resulting JSON in any way, this
will cause selecting from the second view to trigger a segfault.

The solution is easy: regenerate the second view by just CREATE OR
REPLACEing it with the existing definition.

I have created a public Gist with reproduction steps and dumps, attached
here:
https://gist.github.com/samuelhorwitz/0bc77a517238914512fc8cdf50d217cd

Please scroll to the bottom (or click here
https://gist.github.com/samuelhorwitz/0bc77a517238914512fc8cdf50d217cd#gistcomment-2240442)
to see my steps.

This happens in 9.6.5 as well as 10.

Samuel Horwitz


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
@ 2017-10-26 19:09 ` Dmitry Dolgov <9erthalion6@gmail.com>
  1 sibling, 0 replies; 8+ messages in thread

From: Dmitry Dolgov @ 2017-10-26 19:09 UTC (permalink / raw)
  To: samuel.horwitz@gmail.com; +Cc: pgsql-bugs

> On 26 October 2017 at 20:40, <samuel.horwitz@gmail.com> wrote:
>
> However, if I alter the stored procedure's resulting JSON in any way, this
> will cause selecting from the second view to trigger a segfault.

Yes, looks like I can reproduce this issue - from the first glance
parse_relation.c:2216 is the culprit,
somehow aliasp_item is empty:

expandRTE (rte=<optimized out>, rtindex=3, sublevels_up=sublevels_up@entry=0,
location=-1, include_dropped=1 '\001', colnames=colnames@entry=0x7ffda83c0bd0,
colvars=0x7ffda83c0bd8) at parse_relation.c:2216
2216                                                    char       *label =
strVal(lfirst(aliasp_item));
>>> p aliasp_item
$1 = (ListCell *) 0x0

^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
@ 2017-10-26 19:24 ` Michael Paquier <michael.paquier@gmail.com>
  2017-10-26 20:42   ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 8+ messages in thread

From: Michael Paquier @ 2017-10-26 19:24 UTC (permalink / raw)
  To: samuel.horwitz@gmail.com; +Cc: pgsql-bugs

On Thu, Oct 26, 2017 at 11:40 AM,  <samuel.horwitz@gmail.com> wrote:
> I have created a public Gist with reproduction steps and dumps, attached
> here:
> https://gist.github.com/samuelhorwitz/0bc77a517238914512fc8cdf50d217cd
>
> Please scroll to the bottom (or click here
> https://gist.github.com/samuelhorwitz/0bc77a517238914512fc8cdf50d217cd#gistcomment-2240442)
> to see my steps.

I haven't looked at this thing in details. But this data may not be
present forever, say github.com is down or is removed from existence.
So if you can, please always attach any self-contained test case in a
way that it is saved in the archives of postgresql.org. In this case,
that would have been to email directly pgsql-bugs instead of using the
website form.

psql -f initial.sql
# This works
psql -c 'SELECT id, json FROM base_table_json'
psql -f break-it.sql
# this breaks
psql -c 'SELECT id, json FROM base_table_json'
psql -f fix-it.sql
# this works
psql -c 'SELECT id, json FROM base_table_json'

This reminds me of this case:
https://www.postgresql.org/message-id/20150707165212.1188.60819@wrigleys.postgresql.org.
The backtrace is not exactly the same, but that's really close.
-- 
Michael

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Attachments:

  initial.sql (application/octet-stream, 1.4K) download

  fix-it.sql (application/octet-stream, 146B) download

  break-it.sql (application/octet-stream, 383B) download

^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
  2017-10-26 19:24 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
@ 2017-10-26 20:42   ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-26 22:04     ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2017-10-26 20:42 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: pgsql-bugs

Michael Paquier <michael.paquier@gmail.com> writes:
> I haven't looked at this thing in details. But this data may not be
> present forever, say github.com is down or is removed from existence.
> So if you can, please always attach any self-contained test case in a
> way that it is saved in the archives of postgresql.org. In this case,
> that would have been to email directly pgsql-bugs instead of using the
> website form.

Thanks for posting the reproducer.  The attached seems to fix it, but
now that I've seen this, I wonder if there are other similar cases.

			regards, tom lane

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Attachments:

make-expandRTE-cope-with-new-columns.patch (text/x-diff, 1.4K) download diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index ca32a37..dae6fb9 100644 *** a/src/backend/parser/parse_relation.c --- b/src/backend/parser/parse_relation.c *************** expandRTE(RangeTblEntry *rte, int rtinde *** 2207,2217 **** if (colnames) { ! /* Assume there is one alias per target item */ ! char *label = strVal(lfirst(aliasp_item)); *colnames = lappend(*colnames, makeString(pstrdup(label))); - aliasp_item = lnext(aliasp_item); } if (colvars) --- 2207,2235 ---- if (colnames) { ! char *label; ! ! /* ! * We prefer to use the outer query's column aliases ! * as column names. However, in scenarios where ! * columns have been added to a view since the outer ! * query was originally parsed, there could be more ! * tlist items in the subquery than the outer query ! * knew about. In such cases, use the subquery's ! * column names if it has any, else fall back to ! * "?column?". ! */ ! if (aliasp_item) ! { ! label = strVal(lfirst(aliasp_item)); ! aliasp_item = lnext(aliasp_item); ! } ! else if (te->resname) ! label = te->resname; ! else ! label = "?column?"; *colnames = lappend(*colnames, makeString(pstrdup(label))); } if (colvars)
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
  2017-10-26 19:24 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  2017-10-26 20:42   ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-10-26 22:04     ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-28 00:37       ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2017-10-26 22:04 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: pgsql-bugs

I wrote:
> Thanks for posting the reproducer.  The attached seems to fix it, but
> now that I've seen this, I wonder if there are other similar cases.

After some not-very-exhaustive looking around, the only thing I've found
that seems closely related is expandRTE's behavior for a function
returning composite in an RTE_FUNCTION RTE.  It's clearly possible for
somebody to have added columns to the composite type since the calling
query was parsed, so there is a comparable hazard in that case as well.
But what that code path does is to ignore any columns beyond what it
saw originally (which it's memorialized in funccolcount; see the comment
for struct RangeTblFunction).

To be consistent with that, it seems like what the RTE_SUBQUERY case
ought to do is ignore columns beyond the length of eref->colnames.
This is probably less useful than what I posted first --- it means you
don't get to see any added columns in the result of "subqueryname.*".
But I doubt we want different behaviors in the two cases.

			regards, tom lane

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Attachments:

make-expandRTE-ignore-new-columns.patch (text/x-diff, 1.2K) download diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index ca32a37..e89bebf 100644 *** a/src/backend/parser/parse_relation.c --- b/src/backend/parser/parse_relation.c *************** expandRTE(RangeTblEntry *rte, int rtinde *** 2205,2213 **** varattno++; Assert(varattno == te->resno); if (colnames) { - /* Assume there is one alias per target item */ char *label = strVal(lfirst(aliasp_item)); *colnames = lappend(*colnames, makeString(pstrdup(label))); --- 2205,2223 ---- varattno++; Assert(varattno == te->resno); + /* + * In scenarios where columns have been added to a view + * since the outer query was originally parsed, there can + * be more items in the subquery tlist than the outer + * query expects. We should ignore such extra column(s) + * --- compare the behavior for composite-returning + * functions, in the RTE_FUNCTION case below. + */ + if (!aliasp_item) + break; + if (colnames) { char *label = strVal(lfirst(aliasp_item)); *colnames = lappend(*colnames, makeString(pstrdup(label)));
^ permalink raw reply [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
  2017-10-26 19:24 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  2017-10-26 20:42   ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-26 22:04     ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-10-28 00:37       ` Michael Paquier <michael.paquier@gmail.com>
  2017-10-28 01:09         ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 8+ messages in thread

From: Michael Paquier @ 2017-10-28 00:37 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs

On Thu, Oct 26, 2017 at 3:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> To be consistent with that, it seems like what the RTE_SUBQUERY case
> ought to do is ignore columns beyond the length of eref->colnames.
> This is probably less useful than what I posted first --- it means you
> don't get to see any added columns in the result of "subqueryname.*".
> But I doubt we want different behaviors in the two cases.

Sorry for coming up late in the game. I can see that you have pushed a
patch as d5b760e, but back-paddled a bit on d76886c. After some
analysis of things around, I think that you got it right. One comment
I have first though is that you could have used forboth as there is no
point to go through the target list entries once there are no more
aliases. Or target list entries marked as resjunk do not have an
expended reference name?
-- 
Michael


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
  2017-10-26 19:24 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  2017-10-26 20:42   ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-26 22:04     ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-28 00:37       ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
@ 2017-10-28 01:09         ` Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-28 03:14           ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 1 reply; 8+ messages in thread

From: Tom Lane @ 2017-10-28 01:09 UTC (permalink / raw)
  To: Michael Paquier <michael.paquier@gmail.com>; +Cc: pgsql-bugs

Michael Paquier <michael.paquier@gmail.com> writes:
> Sorry for coming up late in the game. I can see that you have pushed a
> patch as d5b760e, but back-paddled a bit on d76886c. After some
> analysis of things around, I think that you got it right. One comment
> I have first though is that you could have used forboth as there is no
> point to go through the target list entries once there are no more
> aliases. Or target list entries marked as resjunk do not have an
> expended reference name?

Right, there's no entry in the outer RTE for resjunk columns.

(In practice, resjunk entries are at the end of the tlist so that it
wouldn't really matter, but I try to keep code from assuming that.)

			regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered
  2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
  2017-10-26 19:24 ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  2017-10-26 20:42   ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-26 22:04     ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
  2017-10-28 00:37       ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Michael Paquier <michael.paquier@gmail.com>
  2017-10-28 01:09         ` Re: BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-10-28 03:14           ` Michael Paquier <michael.paquier@gmail.com>
  0 siblings, 0 replies; 8+ messages in thread

From: Michael Paquier @ 2017-10-28 03:14 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs

On Fri, Oct 27, 2017 at 6:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
>> Sorry for coming up late in the game. I can see that you have pushed a
>> patch as d5b760e, but back-paddled a bit on d76886c. After some
>> analysis of things around, I think that you got it right. One comment
>> I have first though is that you could have used forboth as there is no
>> point to go through the target list entries once there are no more
>> aliases. Or target list entries marked as resjunk do not have an
>> expended reference name?
>
> Right, there's no entry in the outer RTE for resjunk columns.
>
> (In practice, resjunk entries are at the end of the tlist so that it
> wouldn't really matter, but I try to keep code from assuming that.)

OK, thanks for confirming. Yes the current logic is better this way.
-- 
Michael


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs



^ permalink  raw  reply  [nested|flat] 8+ messages in thread


end of thread, other threads:[~2017-10-28 03:14 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-10-26 18:40 BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered samuel.horwitz@gmail.com
2017-10-26 19:09 ` Dmitry Dolgov <9erthalion6@gmail.com>
2017-10-26 19:24 ` Michael Paquier <michael.paquier@gmail.com>
2017-10-26 20:42   ` Tom Lane <tgl@sss.pgh.pa.us>
2017-10-26 22:04     ` Tom Lane <tgl@sss.pgh.pa.us>
2017-10-28 00:37       ` Michael Paquier <michael.paquier@gmail.com>
2017-10-28 01:09         ` Tom Lane <tgl@sss.pgh.pa.us>
2017-10-28 03:14           ` Michael Paquier <michael.paquier@gmail.com>

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