BUG #14876: Segmentation fault with JSONB column used in store proc that gets used by view and later altered

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

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:
/messages/by-id/20150707165212.1188.60819@wrigleys.postgresql.org.
The backtrace is not exactly the same, but that's really close.
--
Michael

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

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

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

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

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