Re: [sqlsmith] Failed assertion in joinrels.c

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-07-26 20:11:50
Message-ID: CA+TgmoZ6rtR0ozn41GB63m1QitKUP45Z9dA3w4apbeHwq8SwYA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 6, 2016 at 3:30 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Sun, Jun 5, 2016 at 11:16 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>>> 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?
>>
>> If we're going to touch this behavior in the back branches, I would
>> vote for changing it the same as we do in HEAD. I do not think that
>> making a user-visible behavior change in a minor release and then a
>> different change in the next major is good strategy.
>
> So, I have finished with the patch attached that changes all the
> *def() functions to return NULL when an object does not exist. Some
> callers of the index and constraint definitions still expect a cache
> lookup error if the object does not exist, and this patch is careful
> about that. I think that it would be nice to get that in 9.6, but I
> won't fight hard for it either. So I am parking it for now in the
> upcoming CF.
>
>> But, given the relative shortage of complaints from the field, I'd
>> be more inclined to just do nothing in back branches. There might
>> be people out there with code depending on the current behavior,
>> and they'd be annoyed if we change it in a minor release.
>
> Sure. That's the safest position. Thinking about the existing behavior
> for some of the index and constraint callers, even just changing the
> error message does not bring much as some of them really expect to
> fail with a cache lookup error.

Committed with minor kibitizing: you don't need an "else" after a
statement that transfers control out of the function. Shouldn't
pg_get_function_arguments, pg_get_function_identity_arguments,
pg_get_function_result, and pg_get_function_arg_default get the same
treatment?

--
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 Robert Haas 2016-07-26 20:21:45 Re: AdvanceXLInsertBuffer vs. WAL segment compressibility
Previous Message Robert Haas 2016-07-26 19:39:35 Re: bug in citext's upgrade script for parallel aggregates