Horton

Thread

Commits

  1. Fix crash when columns have been added to the end of a view.

  1. 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-26T18:40:35Z

    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

  2. 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-26T19:09:00Z

    > 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

  3. 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-26T19:24:06Z

    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

  4. 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-26T20:42:36Z

    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

  5. 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-26T22:04:59Z

    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

  6. 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-28T00:37:29Z

    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

  7. 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-28T01:09:23Z

    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

  8. 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-28T03:14:56Z

    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