Re: [sqlsmith] Failed assertion in joinrels.c

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Andreas Seltenreich <seltenreich(at)gmx(dot)de>, Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sqlsmith] Failed assertion in joinrels.c
Date: 2016-08-17 12:40:48
Message-ID: CA+TgmobCYZKTzMBa2DHEvmbYP_H2wnrQ7NTH0=DapO7c5jLN8g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Aug 17, 2016 at 7:25 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Wed, Aug 10, 2016 at 11:27 AM, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> On Wed, Aug 10, 2016 at 10:04 AM, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>>> This seems better, after checking at other places I found that for
>>> invalid type we are using ERRCODE_UNDEFINED_OBJECT and for invalid
>>> functions we are using ERRCODE_UNDEFINED_FUNCTION. So I have done the
>>> same way.
>>>
>>> Updated patch attached.
>>
>> I found some more places where we can get similar error and updated in
>> my v3 patch.
>>
>
> @@ -412,7 +412,9 @@ plpgsql_validator(PG_FUNCTION_ARGS)
> /* Get the new function's pg_proc entry */
> tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcoid));
> if (!HeapTupleIsValid(tuple))
> - elog(ERROR, "cache lookup failed for function %u", funcoid);
> + ereport(ERROR,
> + (errcode(ERRCODE_UNDEFINED_FUNCTION),
> + errmsg("function with OID %u does not exist", funcoid)));
>
> If you are making changes in plpgsql_validator(), then shouldn't we
> make changes in plperl_validator() or plpython_validator()? I see
> that you have made changes to function CheckFunctionValidatorAccess()
> which seems to be getting called from *_validator() (* indicates
> plpgsql/plperl/plpython) functions. Is there a reason for changing
> the *_validator() function?

Yeah, when I glanced briefly at this patch, I found myself wondering
whether all of these call sites were actually reachable (and why the
patch contained no test cases to prove it).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2016-08-17 12:47:38 drop src/backend/port/darwin/system.c ?
Previous Message Robert Haas 2016-08-17 12:31:36 Re: LWLocks in DSM memory