From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Cache lookup errors with functions manipulation object addresses |
Date: | 2020-03-19 13:30:11 |
Message-ID: | 20200319133011.GV214947@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Mar 19, 2020 at 08:48:58AM +0100, Daniel Gustafsson wrote:
> Taking a look at this to see if we can achieve closure on this long-running
> patchset. The goal of the patch is IMO a clear win for usability.
>
> The patchset applies with a bit of fuzzing and offsetting, it has tests (which
> pass) as well as the relevant documentation changes. I agree with the previous
> reviewers that the new shape of the test is better, so definitely +1 on that
> change. There are a lot of mechanic changes in this set, but AFAICT they cover
> all the bases.
Thanks. I hope it is helpful.
> Since the patch has been through a lot of review already there isn't a lot to
> say, only a few very minor things that stood out:
>
> * This exports the useful functionality of regoperatorout for use
> * in other backend modules. The result is a palloc'd string.
> format_operator_extended has this comment, but the code can now also return
> NULL and not just a palloc'd string.
>
> + if (!missing_ok)
> + {
> + elog(ERROR, "could not find tuple for cast %u",
> + object->objectId);
> + }
> The other (!missing_ok) blocks in this patch are not encapsulating the elog()
> with braces.
>
> I'm switching this to ready for committer,
The new FORMAT_TYPE_* flag still makes sense to me while reading this
code once again, as well as the extensibility of the new APIs for
operators and procedures. One doubt I still have is if we should
really change the signature of getObjectDescription() to include the
new missing_ok argument or if a new function should be introduced.
I'd rather keep only one function as, even if this is called in many
places in the backend, I cannot track down an extension using it, but
I won't go against Alvaro's will either if he thinks something
different as this is his original design and commit as of f8348ea3.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2020-03-19 14:09:45 | Re: Berserk Autovacuum (let's save next Mandrill) |
Previous Message | Mike Palmiotto | 2020-03-19 13:29:53 | Re: Auxiliary Processes and MyAuxProc |