Re: Cache lookup errors with functions manipulation object addresses

On Tue, Jul 07, 2020 at 11:08:30AM -0400, Alvaro Herrera wrote: g> That's a holdover from old times, when we thought functions were > procedures. That's no longer the case. Thanks, so "routine" it is. I have done an extra round of review of this patch, and noticed two things: - In

Re: Cache lookup errors with functions manipulation object addresses

On 2020-Jul-07, Michael Paquier wrote: > On Mon, Jul 06, 2020 at 10:56:53AM -0400, Alvaro Herrera wrote: > > I think "routine" was more correct. We introduced that terminology to > > encompass both functions and procedures, back when real procedures were > > added. See the definitions in the

Re: Cache lookup errors with functions manipulation object addresses

> On 6 Jul 2020, at 09:45, Michael Paquier wrote: > Attached is for now a rebased patch. If there are any comments, > please feel free. Daniel, Alvaro, does that look fine for you? I am > letting this stuff aside for a couple of days for the moment. Reading through I have no comments or

Re: Cache lookup errors with functions manipulation object addresses

On Mon, Jul 06, 2020 at 10:56:53AM -0400, Alvaro Herrera wrote: > I think "routine" was more correct. We introduced that terminology to > encompass both functions and procedures, back when real procedures were > added. See the definitions in the glossary for example. Okay, fine by me, while we

Re: Cache lookup errors with functions manipulation object addresses

On 2020-Jul-06, Michael Paquier wrote: > While refreshing my mind with this code, I got surprised with the > choice of "routine" in getProcedureTypeDescription() when we need a > default object type name for an object not found, so I have switched > that to "procedure" to be more consistent. I

Re: Cache lookup errors with functions manipulation object addresses

On Fri, Jul 03, 2020 at 11:04:17AM -0400, Alvaro Herrera wrote: > 0001 and 0002 look good to me. Thanks for the review. > I think 0003 could be a little more careful about indentation; some long > lines are going to result after pgindent that might be better to handle > in a different way before

Re: Cache lookup errors with functions manipulation object addresses

On Fri, Jul 03, 2020 at 12:34:39PM +0200, Daniel Gustafsson wrote: > No objections from me after skimming over the updated patchset. I still > maintain that the format_operator_extended comment should be amended to > include > that it can return NULL and not alwatys palloced string, but that's

Re: Cache lookup errors with functions manipulation object addresses

On 2020-Jul-02, Michael Paquier wrote: > Attached is an updated patch set, because of conflicts in the docs. > Daniel, you are still registered as a reviewer of this patch, and it > is marked as ready for committer? Do you have more comments? Would > anybody have objections if I move on with

Re: Cache lookup errors with functions manipulation object addresses

> On 2 Jul 2020, at 07:35, Michael Paquier wrote: > > On Thu, Mar 19, 2020 at 10:30:11PM +0900, Michael Paquier wrote: >> 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

Re: Cache lookup errors with functions manipulation object addresses

On Thu, Mar 19, 2020 at 10:30:11PM +0900, Michael Paquier wrote: > 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

Re: Cache lookup errors with functions manipulation object addresses

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 >

Re: Cache lookup errors with functions manipulation object addresses

> On 17 Oct 2019, at 03:37, Michael Paquier wrote: > Attached is an updated > patch set with the heap_close() calls removed as per the recent report > from Dmitry. 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

Re: Cache lookup errors with functions manipulation object addresses

On Thu, Sep 26, 2019 at 03:52:03PM +0900, Michael Paquier wrote: > On Wed, Sep 25, 2019 at 09:21:03AM -0300, Alvaro Herrera wrote: >> On 2019-Sep-24, Michael Paquier wrote: >>> + * - FORMAT_TYPE_FORCE_NULL >>> + * if the type OID is invalid or unknown, return NULL instead of ??? >>> + * or

Re: Cache lookup errors with functions manipulation object addresses

On Wed, Oct 16, 2019 at 01:04:57PM +0200, Dmitry Dolgov wrote: > Thanks for the update. Looking at v17 0003 I have one more question. In all > the > places where we have to do systable_endscan, it followed by heap_close, > although in the rest of the file table_close is used. I guess this logic

Re: Cache lookup errors with functions manipulation object addresses

> On Tue, Sep 24, 2019 at 1:58 AM Michael Paquier wrote: > > Please feel free to use the updated versions attached. These can > apply on top of HEAD at 30d1379. Thanks for the update. Looking at v17 0003 I have one more question. In all the places where we have to do systable_endscan, it

Re: Cache lookup errors with functions manipulation object addresses

On Wed, Sep 25, 2019 at 09:21:03AM -0300, Alvaro Herrera wrote: > On 2019-Sep-24, Michael Paquier wrote: >> + * - FORMAT_TYPE_FORCE_NULL >> + *if the type OID is invalid or unknown, return NULL instead of ??? >> + *or such > > I think FORCE_NULL is a strange name for this flag (and its two

Re: Cache lookup errors with functions manipulation object addresses

On 2019-Sep-25, Michael Paquier wrote: > On Tue, Sep 24, 2019 at 03:25:08PM +0900, Kyotaro Horiguchi wrote: > > In 0003, empty string and NULL are not digtinguishable in psql > > text output. It'd be better that the regression test checks that > > the return is actually NULL using "is NULL" or

Re: Cache lookup errors with functions manipulation object addresses

On Tue, Sep 24, 2019 at 03:25:08PM +0900, Kyotaro Horiguchi wrote: > In 0003, empty string and NULL are not digtinguishable in psql > text output. It'd be better that the regression test checks that > the return is actually NULL using "is NULL" or "\pset null > ''" or something like. For a patch

Re: Cache lookup errors with functions manipulation object addresses

At Tue, 24 Sep 2019 08:58:50 +0900, Michael Paquier wrote in <20190923235850.ga2...@paquier.xyz> > On Mon, Sep 23, 2019 at 09:15:24PM +0200, Dmitry Dolgov wrote: > Please feel free to use the updated versions attached. These can > apply on top of HEAD at 30d1379. In 0003, empty string and NULL

Re: Cache lookup errors with functions manipulation object addresses

On Mon, Sep 23, 2019 at 09:15:24PM +0200, Dmitry Dolgov wrote: > Thanks for the patch! I couldn't check it in action, since looks like it > doesn't apply anymore [1] (although after a quick check I'm not entirely sure > why). Nevertheless I have a few short commentaries: Thanks for the review.

Re: Cache lookup errors with functions manipulation object addresses

> On Tue, Jul 2, 2019 at 9:28 AM Michael Paquier wrote: > > On Thu, Feb 21, 2019 at 04:40:13PM +0900, Michael Paquier wrote: > > Rebased version fixing some conflicts with HEAD. > > And rebased version for this stuff on HEAD (66c5bd3), giving visibly > v16. Thanks for the patch! I couldn't check

Re: Cache lookup errors with functions manipulation object addresses

On Thu, Feb 21, 2019 at 04:40:13PM +0900, Michael Paquier wrote: > Rebased version fixing some conflicts with HEAD. And rebased version for this stuff on HEAD (66c5bd3), giving visibly v16. -- Michael From 76ef961e02e0276af5516bdce6e0e543526db5b2 Mon Sep 17 00:00:00 2001 From: Michael Paquier

Re: Cache lookup errors with functions manipulation object addresses

On Thu, Jan 31, 2019 at 04:08:18PM +0900, Michael Paquier wrote: > End-of-commit-fest rebase and moved to next CF. Rebased version fixing some conflicts with HEAD. -- Michael From 5844b0ba07fdd8373a7ef15a1676178cae53c40d Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 25 Dec 2018

Re: Cache lookup errors with functions manipulation object addresses

On Tue, Dec 25, 2018 at 01:34:38PM +0900, Michael Paquier wrote: > With that all the regression tests show a consistent behavior for all > object types when those are undefined. Thoughts? End-of-commit-fest rebase and moved to next CF. -- Michael From 6514d737cef100ea8146b1d6d10fa93e40c6ee9c Mon

Re: Cache lookup errors with functions manipulation object addresses

Álvaro, all, On Fri, Dec 14, 2018 at 09:04:36AM +0900, Michael Paquier wrote: > Thanks again for looking up at what was proposed. I'll see if I can > finish the refactoring part for the next CF, and be done with this > stuff. Please find attached an updated patch set which reworks the SQL

Re: Cache lookup errors with functions manipulation object addresses

On Thu, Dec 13, 2018 at 02:58:02PM -0300, Alvaro Herrera wrote: > On 2018-Dec-13, Michael Paquier wrote: >> Attached is an updated version for that as 0001. Thanks for the >> review. Does that part look good to you now? > > +1. Thanks for the review, I have applied this part. > Hmm ...

Re: Cache lookup errors with functions manipulation object addresses

On 2018-Dec-13, Michael Paquier wrote: > > I support 0001 other than those two points. > > Attached is an updated version for that as 0001. Thanks for the > review. Does that part look good to you now? +1. > > +SELECT * FROM pg_identify_object_as_address('pg_proc'::regclass, 0, 0); -- > >

Re: Cache lookup errors with functions manipulation object addresses

On Wed, Dec 12, 2018 at 02:21:29PM -0300, Alvaro Herrera wrote: > I think defining bit flags in an enum is weird; I'd use defines > instead. Okay, I am fine with that. > And (a purely stylistic point) I'd use bits32 rather than uint32. (I'm > probably alone in this opinion, since almost every

Re: Cache lookup errors with functions manipulation object addresses

On 2018-Sep-18, Michael Paquier wrote: > On Fri, Sep 14, 2018 at 12:07:23PM -0300, Alvaro Herrera wrote: > > On 2018-Sep-14, Alvaro Herrera wrote: > >> I haven't looked at 0003 yet. > > Thanks for the review. I have pushed 0002 for now. I had one doubt > about 0001 though. So as to avoid

Re: Cache lookup errors with functions manipulation object addresses

On Fri, Sep 14, 2018 at 12:07:23PM -0300, Alvaro Herrera wrote: > On 2018-Sep-14, Alvaro Herrera wrote: >> I haven't looked at 0003 yet. Thanks for the review. I have pushed 0002 for now. I had one doubt about 0001 though. So as to avoid redesigning the APIs for FDWs and servers again in the

Re: Cache lookup errors with functions manipulation object addresses

On 2018-Sep-14, Alvaro Herrera wrote: > I haven't looked at 0003 yet. It's strange that pg_identify_object returns empty type in only some cases (as seen in the regression test output) ... and this one definitely does not make sense: +SELECT * FROM pg_identify_object('pg_class'::regclass,

Re: Cache lookup errors with functions manipulation object addresses

On 2018-Sep-14, Michael Paquier wrote: > On Fri, Sep 14, 2018 at 11:22:12AM +0900, Michael Paquier wrote: > > Attached are rebased versions. This has been around for some time, so I > > am planning to move on with this patch set pretty soon as that's mainly > > cleanup for getObjectIdentity as

Re: Cache lookup errors with functions manipulation object addresses

On Fri, Sep 14, 2018 at 11:22:12AM +0900, Michael Paquier wrote: > Attached are rebased versions. This has been around for some time, so I > am planning to move on with this patch set pretty soon as that's mainly > cleanup for getObjectIdentity as it triggers elog("cache lookup") or > such for

Re: Cache lookup errors with functions manipulation object addresses

On Mon, Jul 02, 2018 at 08:54:25PM +0900, Michael Paquier wrote: > I am fine with any conclusion. As the patch has rotten a bit, I > switched it from "Ready for committer" to "Needs Review" by the way. > That seems more appropriate as the thing has rotten a bit. Attached are rebased versions.

Re: Cache lookup errors with functions manipulation object addresses

On Sun, Jul 01, 2018 at 12:31:17PM -0400, Andrew Dunstan wrote: > I think you're asserting far too broad a policy for the CF, and in any case > there has been no discussion of what exactly is a large patch. I don't see > any great need to defer patch 3. It is substantial although not what I would

Re: Cache lookup errors with functions manipulation object addresses

On 07/01/2018 10:27 AM, Michael Paquier wrote: On Wed, May 16, 2018 at 01:03:18PM +0900, Michael Paquier wrote: Okay, I have done so in the updated set attached. I have added some documentation as well in fdwhandler.sgml about those two new things. That's too late for v11 of course, so

Re: Cache lookup errors with functions manipulation object addresses

On Wed, May 16, 2018 at 01:03:18PM +0900, Michael Paquier wrote: > Okay, I have done so in the updated set attached. I have added some > documentation as well in fdwhandler.sgml about those two new things. > That's too late for v11 of course, so let's them sit until the time > comes. Attached

Re: Cache lookup errors with functions manipulation object addresses

On Mon, May 14, 2018 at 06:32:52PM -0400, Alvaro Herrera wrote: > So, I don't know -- if the reaction is to add a #ifdef for pg version > that adds a second argument passed always false, then we haven't won > anything. Yeah, that improves nothing.. >> What about naming those

Re: Cache lookup errors with functions manipulation object addresses

On 2018-May-15, Michael Paquier wrote: > On Mon, May 14, 2018 at 04:27:48PM -0400, Alvaro Herrera wrote: > > I think we're better off adding a new function and avoid changing the > > signature of GetForeignServer et al. Or maybe rename the function and > > keep the original name as a

Re: Cache lookup errors with functions manipulation object addresses

On Mon, May 14, 2018 at 04:27:48PM -0400, Alvaro Herrera wrote: > I think we're better off adding a new function and avoid changing the > signature of GetForeignServer et al. Or maybe rename the function and > keep the original name as a compatibility wrapper macro. On the other hand, if we make

Re: Cache lookup errors with functions manipulation object addresses

On 2018-Mar-06, Michael Paquier wrote: > On Mon, Mar 05, 2018 at 12:57:38PM +, Aleksander Alekseev wrote: > > It looks like that this patch doesn't apply anymore: > > http://commitfest.cputube.org/ > > > > The new status of this patch is: Waiting on Author > > Yes, this happens because

Re: Cache lookup errors with functions manipulation object addresses

On Mon, Mar 05, 2018 at 12:57:38PM +, Aleksander Alekseev wrote: > It looks like that this patch doesn't apply anymore: > http://commitfest.cputube.org/ > > The new status of this patch is: Waiting on Author Yes, this happens because patch 0003 from the last series has been committed as

Re: Cache lookup errors with functions manipulation object addresses

The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested Hello Michael, It looks like that this patch doesn't apply anymore:

Re: [HACKERS] Cache lookup errors with functions manipulation object addresses

On Sat, Feb 17, 2018 at 07:17:24PM -0300, Alvaro Herrera wrote: > Pushed 0003. Thanks. > Maybe we can get rid of format_type_be_qualified too, but I didn't > care too much about it either; it's not a big deal I think. Agreed. There are 16 callers spread in objectaddress.c and regproc.c, this

Re: [HACKERS] Cache lookup errors with functions manipulation object addresses

Michael Paquier wrote: > > As far as format_type_extended() is concerned, IMO we've gone far enough > > with the number of variants of format_type(). Making the function > > public makes sense to me, but let's add a bits32 flags argument instead > > of exposing the messy set of booleans. We can

Re: [HACKERS] Cache lookup errors with functions manipulation object addresses

On Mon, Nov 27, 2017 at 1:01 AM, Michael Paquier wrote: > On Fri, Nov 24, 2017 at 9:13 PM, Michael Paquier > wrote: >> Attached is a rebased patch set. Álvaro, as you have introduced most >> of the problems with 4464303 & friends dated of