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-06 07:30:22
Message-ID: CAB7nPqTxF5dtxjEzB7xkJvOWxX8D_2atxmTu3PSnkhcWT_JY5A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
def-funcs-null.patch text/x-diff 13.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-06-06 08:04:54 installcheck failing on psql_crosstab
Previous Message Albe Laurenz 2016-06-06 07:19:37 Re: Prepared statements and generic plans