Re: BUG #14220: pg_get_expr() with an incorrect relation id crashes the server

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: christopher(dot)m(dot)hanks(at)gmail(dot)com, PostgreSQL Bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #14220: pg_get_expr() with an incorrect relation id crashes the server
Date: 2016-07-01 01:12:06
Message-ID: CAB7nPqRhsKK8BP1yo=s4ZKH42gzSwTfDWHVT2C_PbK6cqzTkJg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Fri, Jul 1, 2016 at 5:42 AM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> Maybe the attnum bounds check should be an error rather than an
> assertion, like in the attached. Thought perhaps with a better
> message...

Quite interesting report. It looks right to me to do so with an elog(ERROR).

+ if (attnum > colinfo->num_cols)
+ elog(ERROR, "cannot decompile bogus attnum: %d", attnum);
Why not "incorrect number of arguments: specified %d but found %d" instead?

I have poked at this bug report yesterday and today, and found that it
is as well possible to hit the second assertion two lines below with a
dropped column. So I would like to propose the patch attached to
replace both assertions with an elog(ERROR). Test cases are included,
though I am not sure if they bring much value.
--
Michael

Attachment Content-Type Size
reject-bogus-attnum-v2.patch text/x-diff 3.0 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message lishengxian123 2016-07-01 03:30:18 BUG #14221: using postgresql 9.1.22 to 9.3.13 failed using pg_upgrade
Previous Message Michael Paquier 2016-07-01 00:59:45 Re: Postgres 9.5 Installation on OS X