Re: [sqlsmith] Failed assertion in joinrels.c

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: 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-06-05 09:29:23
Message-ID: CAB7nPqTfzOVgdSK3jKM2L1Awy9nV6RQQni09f8cHjtny6jeQOw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Jun 5, 2016 at 4:28 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sun, Jun 5, 2016 at 12:29 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>>> This is still failing:
>>> =# select indexdef from pg_catalog.pg_indexes where indexdef is not NULL;
>>> ERROR: XX000: cache lookup failed for index 2619
>>> LOCATION: pg_get_indexdef_worker, ruleutils.c:1054
>>> Do we want to improve at least the error message?
>>
>> Maybe we should just make the function silently return NULL if the OID
>> doesn't refer to an index relation. There's precedent for that sort
>> of behavior ...
>
> For views it is simply returned "Not a view", and for rules that's
> "-". All the others behave like similarly to indexes:
> =# select pg_get_constraintdef(0);
> ERROR: XX000: cache lookup failed for constraint 0
> =# select pg_get_triggerdef(0);
> ERROR: XX000: could not find tuple for trigger 0
> =# select pg_get_functiondef(0);
> ERROR: XX000: cache lookup failed for function 0
> =# select pg_get_viewdef(0);
> pg_get_viewdef
> ----------------
> Not a view
> (1 row)
> =# select pg_get_ruledef(0);
> pg_get_ruledef
> ----------------
> -
> (1 row)
>
> So yes we could just return NULL for all of them, or make them throw
> an error. Anyway, my vote goes for a HEAD-only change, and just throw
> NULL... This way an application still has the option to fallback to
> something else with a CASE at SQL level without using plpgsql to catch
> the error.
>
> Also, I have not monitored the code much regarding some code paths
> that rely on the existing rules, but I would imagine that there are
> things depending on the current behavior as well.

Still, on back branches I think that it would be a good idea to have a
better error message for the ones that already throw an error, like
"trigger with OID %u does not exist". Switching from elog to ereport
would be a good idea, but wouldn't it be a problem to change what is
user-visible?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tatsuo Ishii 2016-06-05 13:11:10 Re: Statement timeout
Previous Message Michael Paquier 2016-06-05 07:28:09 Re: [sqlsmith] Failed assertion in joinrels.c