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

On Thu, Jun 30, 2016 at 10:43 AM, <christopher.m.hanks@gmail.com> wrote:

The following bug has been logged on the website:

Bug reference: 14220
Logged by: Chris Hanks
Email address: christopher.m.hanks@gmail.com
PostgreSQL version: 9.5.3
Operating system: Linux Mint 17.2 64-bit
Description:

SELECT version():
"PostgreSQL 9.5.3 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu
4.8.2-19ubuntu1) 4.8.2, 64-bit"

Reproduction:

CREATE TABLE "test_table" ("id" serial PRIMARY KEY, "column_1" int4 NOT
NULL, "column_2" int4);

CREATE INDEX "idx" ON "test_table" ("column_1") WHERE ("column_2" = 4);

SELECT pg_get_expr(i.indpred, i.indexrelid) FROM pg_index i;

I realize that the query is incorrect, in order to retrieve the filter
expression as SQL I need to call pg_get_expr(i.indpred, i.indrelid). But it
seems like it should raise an error, rather than causing the server to
crash.

Maybe the attnum bounds check should be an error rather than an
assertion, like in the attached. Thought perhaps with a better
message...

--
Thomas Munro
http://www.enterprisedb.com

On Fri, Jul 1, 2016 at 5:42 AM, Thomas Munro
<thomas.munro@enterprisedb.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

Michael Paquier <michael.paquier@gmail.com> writes:

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).

There's a quite similar error check just a few lines above that prints

elog(ERROR, "bogus varattno for subquery var: %d", var->varattno);

I'm not necessarily wedded to that exact phrasing, but I think these new
error messages should be consistent with that one. I'm a bit inclined
to make all three of them read like

elog(ERROR, "invalid attnum %d for table \"%s\"",
attnum, rte->eref->aliasname);

Test cases are included,
though I am not sure if they bring much value.

Yeah, I can't get very excited about memorializing those.

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