Thread

1. [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2011-06-14T12:06:33Z

   The attached patch is a preparation to rework implementation of DROP statement
   using a common code. That intends to apply get_object_address() to resolve name
   of objects to be removed, and eventually minimizes the number of places to put
   permission checks.
   
   Its first step is an enhancement of get_object_address; to accept 'missing_ok'
   argument to handle cases when IF EXISTS clause is supplied.
   If 'missing_ok' was true and the supplied name was not found, the patched
   get_object_address() returns an ObjectAddress with InvalidOid as objectId.
   If 'missing_ok' was false, its behavior is not changed.
   
   Thanks,
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   

2. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Robert Haas <robertmhaas@gmail.com> — 2011-06-19T02:51:53Z

   On Tue, Jun 14, 2011 at 8:06 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   > The attached patch is a preparation to rework implementation of DROP statement
   > using a common code. That intends to apply get_object_address() to resolve name
   > of objects to be removed, and eventually minimizes the number of places to put
   > permission checks.
   >
   > Its first step is an enhancement of get_object_address; to accept 'missing_ok'
   > argument to handle cases when IF EXISTS clause is supplied.
   > If 'missing_ok' was true and the supplied name was not found, the patched
   > get_object_address() returns an ObjectAddress with InvalidOid as objectId.
   > If 'missing_ok' was false, its behavior is not changed.
   
   Let's consistently make missing_ok the last argument to all of the
   functions to which we're adding it.
   
   I don't think it's a good idea for get_relation_by_qualified_name() to
   be second-guessing the error message that RangeVarGetRelid() feels
   like throwing.
   
   I think that attempting to fetch the column foo.bar when foo doesn't
   exist should be an error even if missing_ok is true.  I believe that's
   consistent with what we do elsewhere.  (If it *were* necessary to open
   the relation without failing if it's not there, you could use
   try_relation_openrv instead of doing as you've done here.)
   
   There is certainly a more compact way of writing the logic in
   get_object_address_typeobj.  Also, I think that function should be
   called get_object_address_type(); the "obj" on the end seems
   redundant.
   
   Apart from those comments this looks pretty sensible.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   
   

3. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2011-06-19T08:48:03Z

   Thanks for your review.
   
   2011/6/19 Robert Haas <robertmhaas@gmail.com>:
   > On Tue, Jun 14, 2011 at 8:06 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   >> The attached patch is a preparation to rework implementation of DROP statement
   >> using a common code. That intends to apply get_object_address() to resolve name
   >> of objects to be removed, and eventually minimizes the number of places to put
   >> permission checks.
   >>
   >> Its first step is an enhancement of get_object_address; to accept 'missing_ok'
   >> argument to handle cases when IF EXISTS clause is supplied.
   >> If 'missing_ok' was true and the supplied name was not found, the patched
   >> get_object_address() returns an ObjectAddress with InvalidOid as objectId.
   >> If 'missing_ok' was false, its behavior is not changed.
   >
   > Let's consistently make missing_ok the last argument to all of the
   > functions to which we're adding it.
   >
   OK. I revised position of the 'missing_ok' argument.
   
   > I don't think it's a good idea for get_relation_by_qualified_name() to
   > be second-guessing the error message that RangeVarGetRelid() feels
   > like throwing.
   >
   OK. I revised the patch to provide 'true' on RangeVarGetRelid().
   Its side effect is on error messages when user gives undefined object name.
   
   > I think that attempting to fetch the column foo.bar when foo doesn't
   > exist should be an error even if missing_ok is true.  I believe that's
   > consistent with what we do elsewhere.  (If it *were* necessary to open
   > the relation without failing if it's not there, you could use
   > try_relation_openrv instead of doing as you've done here.)
   >
   It was fixed. AlterTable() already open the relation (without missing_ok)
   in the case when we drop a column, so I don't think we need to acquire
   relation locks if the supplied column was missing.
   
   > There is certainly a more compact way of writing the logic in
   > get_object_address_typeobj.  Also, I think that function should be
   > called get_object_address_type(); the "obj" on the end seems
   > redundant.
   >
   I renamed the function name to get_object_address_type(), and
   consolidate initialization of ObjectAddress variables.
   
   Thanks,
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   

4. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2011-06-19T11:40:28Z

   Sorry, the previous revision did not update regression test part
   towards the latest one.
   
   2011/6/19 Kohei KaiGai <kaigai@kaigai.gr.jp>:
   > Thanks for your review.
   >
   > 2011/6/19 Robert Haas <robertmhaas@gmail.com>:
   >> On Tue, Jun 14, 2011 at 8:06 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   >>> The attached patch is a preparation to rework implementation of DROP statement
   >>> using a common code. That intends to apply get_object_address() to resolve name
   >>> of objects to be removed, and eventually minimizes the number of places to put
   >>> permission checks.
   >>>
   >>> Its first step is an enhancement of get_object_address; to accept 'missing_ok'
   >>> argument to handle cases when IF EXISTS clause is supplied.
   >>> If 'missing_ok' was true and the supplied name was not found, the patched
   >>> get_object_address() returns an ObjectAddress with InvalidOid as objectId.
   >>> If 'missing_ok' was false, its behavior is not changed.
   >>
   >> Let's consistently make missing_ok the last argument to all of the
   >> functions to which we're adding it.
   >>
   > OK. I revised position of the 'missing_ok' argument.
   >
   >> I don't think it's a good idea for get_relation_by_qualified_name() to
   >> be second-guessing the error message that RangeVarGetRelid() feels
   >> like throwing.
   >>
   > OK. I revised the patch to provide 'true' on RangeVarGetRelid().
   > Its side effect is on error messages when user gives undefined object name.
   >
   >> I think that attempting to fetch the column foo.bar when foo doesn't
   >> exist should be an error even if missing_ok is true.  I believe that's
   >> consistent with what we do elsewhere.  (If it *were* necessary to open
   >> the relation without failing if it's not there, you could use
   >> try_relation_openrv instead of doing as you've done here.)
   >>
   > It was fixed. AlterTable() already open the relation (without missing_ok)
   > in the case when we drop a column, so I don't think we need to acquire
   > relation locks if the supplied column was missing.
   >
   >> There is certainly a more compact way of writing the logic in
   >> get_object_address_typeobj.  Also, I think that function should be
   >> called get_object_address_type(); the "obj" on the end seems
   >> redundant.
   >>
   > I renamed the function name to get_object_address_type(), and
   > consolidate initialization of ObjectAddress variables.
   >
   > Thanks,
   > --
   > KaiGai Kohei <kaigai@kaigai.gr.jp>
   >
   
   
   
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   

5. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Robert Haas <robertmhaas@gmail.com> — 2011-06-22T01:50:14Z

   On Sun, Jun 19, 2011 at 7:40 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   > Sorry, the previous revision did not update regression test part
   > towards the latest one.
   
   Some of the refactoring you've done here seems likely to break things,
   because you're basically making the relation locking happen later than
   it does not, and that's going to cause problems.
   get_object_address_relobject() is a particularly egregious
   rearrangement.  It seems to me that the right formula is to call
   relation_openrv() if missing_ok is false, and try_relation_openrv() if
   missing_ok is true.  But that's sort of a pain, so I propose to first
   apply the attached patch, which gets rid of try_relation_openrv() and
   try_heap_openrv() and instead adds a missing_ok argument to
   relation_openrv() and heap_openrv().  If we do this, then the
   missing_ok argument can just be passed through all the way down.
   
   Thoughts?  Comments?  Objections?
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   

6. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-22T03:04:51Z

   Robert Haas <robertmhaas@gmail.com> writes:
   > Some of the refactoring you've done here seems likely to break things,
   > because you're basically making the relation locking happen later than
   > it does not, and that's going to cause problems.
   > get_object_address_relobject() is a particularly egregious
   > rearrangement.  It seems to me that the right formula is to call
   > relation_openrv() if missing_ok is false, and try_relation_openrv() if
   > missing_ok is true.  But that's sort of a pain, so I propose to first
   > apply the attached patch, which gets rid of try_relation_openrv() and
   > try_heap_openrv() and instead adds a missing_ok argument to
   > relation_openrv() and heap_openrv().  If we do this, then the
   > missing_ok argument can just be passed through all the way down.
   
   > Thoughts?  Comments?  Objections?
   
   At least the last hunk (in pltcl.c) seems to have the flag backwards.
   
   			regards, tom lane
   
   

7. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Robert Haas <robertmhaas@gmail.com> — 2011-06-22T03:11:41Z

   On Tue, Jun 21, 2011 at 11:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   > Robert Haas <robertmhaas@gmail.com> writes:
   >> Some of the refactoring you've done here seems likely to break things,
   >> because you're basically making the relation locking happen later than
   >> it does not, and that's going to cause problems.
   >> get_object_address_relobject() is a particularly egregious
   >> rearrangement.  It seems to me that the right formula is to call
   >> relation_openrv() if missing_ok is false, and try_relation_openrv() if
   >> missing_ok is true.  But that's sort of a pain, so I propose to first
   >> apply the attached patch, which gets rid of try_relation_openrv() and
   >> try_heap_openrv() and instead adds a missing_ok argument to
   >> relation_openrv() and heap_openrv().  If we do this, then the
   >> missing_ok argument can just be passed through all the way down.
   >
   >> Thoughts?  Comments?  Objections?
   >
   > At least the last hunk (in pltcl.c) seems to have the flag backwards.
   
   Ah, nuts.  Sorry.  I think that and parse_relation.c are the only
   places where the try variants are used; nobody else is willing to
   fail, and everyone else is passing false.
   
   Revised patch attached.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   

8. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Noah Misch <noah@leadboat.com> — 2011-06-22T10:18:08Z

   On Tue, Jun 21, 2011 at 11:11:41PM -0400, Robert Haas wrote:
   > On Tue, Jun 21, 2011 at 11:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   > > Robert Haas <robertmhaas@gmail.com> writes:
   > >> Some of the refactoring you've done here seems likely to break things,
   > >> because you're basically making the relation locking happen later than
   > >> it does not, and that's going to cause problems.
   > >> get_object_address_relobject() is a particularly egregious
   > >> rearrangement. ?It seems to me that the right formula is to call
   > >> relation_openrv() if missing_ok is false, and try_relation_openrv() if
   > >> missing_ok is true. ?But that's sort of a pain, so I propose to first
   > >> apply the attached patch, which gets rid of try_relation_openrv() and
   > >> try_heap_openrv() and instead adds a missing_ok argument to
   > >> relation_openrv() and heap_openrv(). ?If we do this, then the
   > >> missing_ok argument can just be passed through all the way down.
   > >
   > >> Thoughts? ?Comments? ?Objections?
   > >
   > > At least the last hunk (in pltcl.c) seems to have the flag backwards.
   > 
   > Ah, nuts.  Sorry.  I think that and parse_relation.c are the only
   > places where the try variants are used; nobody else is willing to
   > fail, and everyone else is passing false.
   > 
   > Revised patch attached.
   
   All existing call sites updated by this patch hardcode the flag, and only 3?
   proposed call sites would take advantage of the ability to not do so.  The
   try_relation_openrv() case is fairly rare and likely to remain that way.  Given
   that, I mildly prefer the code as it is, even if that means doing "missing_ok ?
   try_relation_openrv() : relation_openrv()" in a few places.  Could always wrap
   that in a static function of objectaddress.c.
   
   

9. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

   Robert Haas <robertmhaas@gmail.com> — 2011-06-22T12:56:02Z

   On Wed, Jun 22, 2011 at 6:18 AM, Noah Misch <noah@leadboat.com> wrote:
   > On Tue, Jun 21, 2011 at 11:11:41PM -0400, Robert Haas wrote:
   >> On Tue, Jun 21, 2011 at 11:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
   >> > Robert Haas <robertmhaas@gmail.com> writes:
   >> >> Some of the refactoring you've done here seems likely to break things,
   >> >> because you're basically making the relation locking happen later than
   >> >> it does not, and that's going to cause problems.
   >> >> get_object_address_relobject() is a particularly egregious
   >> >> rearrangement. ?It seems to me that the right formula is to call
   >> >> relation_openrv() if missing_ok is false, and try_relation_openrv() if
   >> >> missing_ok is true. ?But that's sort of a pain, so I propose to first
   >> >> apply the attached patch, which gets rid of try_relation_openrv() and
   >> >> try_heap_openrv() and instead adds a missing_ok argument to
   >> >> relation_openrv() and heap_openrv(). ?If we do this, then the
   >> >> missing_ok argument can just be passed through all the way down.
   >> >
   >> >> Thoughts? ?Comments? ?Objections?
   >> >
   >> > At least the last hunk (in pltcl.c) seems to have the flag backwards.
   >>
   >> Ah, nuts.  Sorry.  I think that and parse_relation.c are the only
   >> places where the try variants are used; nobody else is willing to
   >> fail, and everyone else is passing false.
   >>
   >> Revised patch attached.
   >
   > All existing call sites updated by this patch hardcode the flag, and only 3?
   > proposed call sites would take advantage of the ability to not do so.  The
   > try_relation_openrv() case is fairly rare and likely to remain that way.  Given
   > that, I mildly prefer the code as it is, even if that means doing "missing_ok ?
   > try_relation_openrv() : relation_openrv()" in a few places.  Could always wrap
   > that in a static function of objectaddress.c.
   
   I don't really like the idea of having a static function in
   objectaddress.c, because I think there will eventually be more callers
   who sometimes want to pass missing_ok = true and other times pass
   missing_ok = false.  Plus, it seems a little nutty to have a function
   that, depending on whether its argument is true or false, calls one of
   two other functions which are virtually cut-and-paste copies of each
   other, except that one internally has true and the other false.  I
   just work here, though.
   
   Another option might be to leave heap_openrv() and relation_openrv()
   alone and add a missing_ok argument to try_heap_openrv() and
   try_relation_openrv().  Passing true would give the same behavior as
   presently; passing false would make them behave like the non-try
   version.
   
   -- 
   Robert Haas
   EnterpriseDB: http://www.enterprisedb.com
   The Enterprise PostgreSQL Company
   
   

10. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-22T14:17:09Z

    Robert Haas <robertmhaas@gmail.com> writes:
    > Another option might be to leave heap_openrv() and relation_openrv()
    > alone and add a missing_ok argument to try_heap_openrv() and
    > try_relation_openrv().
    
    +1 for that, although the try_ prefix might be inappropriate naming
    now; how about cond_relation_openrv?
    
    			regards, tom lane
    
    

11. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Alvaro Herrera <alvherre@commandprompt.com> — 2011-06-22T16:51:18Z

    Excerpts from Robert Haas's message of mié jun 22 08:56:02 -0400 2011:
    
    > Another option might be to leave heap_openrv() and relation_openrv()
    > alone and add a missing_ok argument to try_heap_openrv() and
    > try_relation_openrv().  Passing true would give the same behavior as
    > presently; passing false would make them behave like the non-try
    > version.
    
    That would be pretty weird, having two functions, one of them sometimes
    doing the same thing as the other one.
    
    I understand Noah's concern but I think your original proposal was saner
    than both options presented so far.
    
    -- 
    Álvaro Herrera <alvherre@commandprompt.com>
    The PostgreSQL Company - Command Prompt, Inc.
    PostgreSQL Replication, Consulting, Custom Development, 24x7 support
    
    

12. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Robert Haas <robertmhaas@gmail.com> — 2011-06-22T17:36:00Z

    On Wed, Jun 22, 2011 at 12:51 PM, Alvaro Herrera
    <alvherre@commandprompt.com> wrote:
    > Excerpts from Robert Haas's message of mié jun 22 08:56:02 -0400 2011:
    >
    >> Another option might be to leave heap_openrv() and relation_openrv()
    >> alone and add a missing_ok argument to try_heap_openrv() and
    >> try_relation_openrv().  Passing true would give the same behavior as
    >> presently; passing false would make them behave like the non-try
    >> version.
    >
    > That would be pretty weird, having two functions, one of them sometimes
    > doing the same thing as the other one.
    >
    > I understand Noah's concern but I think your original proposal was saner
    > than both options presented so far.
    
    I agree with you.  If we had a whole pile of options it might be worth
    having heap_openrv() and heap_openrv_extended() so as not to
    complicate the simple case, but since there's no forseeable need to
    add anything other than missing_ok, my gut is to just add it and call
    it good.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    

13. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Kohei KaiGai <kaigai@kaigai.gr.jp> — 2011-06-23T09:51:55Z

    I revised my patch based on your "there-is-no-try-v2.patch".
    It enabled to implement 'missing_ok' support without modification of
    orders to solve the object name and relation locking.
    
    Thanks,
    
    2011/6/22 Robert Haas <robertmhaas@gmail.com>:
    > On Wed, Jun 22, 2011 at 12:51 PM, Alvaro Herrera
    > <alvherre@commandprompt.com> wrote:
    >> Excerpts from Robert Haas's message of mié jun 22 08:56:02 -0400 2011:
    >>
    >>> Another option might be to leave heap_openrv() and relation_openrv()
    >>> alone and add a missing_ok argument to try_heap_openrv() and
    >>> try_relation_openrv().  Passing true would give the same behavior as
    >>> presently; passing false would make them behave like the non-try
    >>> version.
    >>
    >> That would be pretty weird, having two functions, one of them sometimes
    >> doing the same thing as the other one.
    >>
    >> I understand Noah's concern but I think your original proposal was saner
    >> than both options presented so far.
    >
    > I agree with you.  If we had a whole pile of options it might be worth
    > having heap_openrv() and heap_openrv_extended() so as not to
    > complicate the simple case, but since there's no forseeable need to
    > add anything other than missing_ok, my gut is to just add it and call
    > it good.
    >
    > --
    > Robert Haas
    > EnterpriseDB: http://www.enterprisedb.com
    > The Enterprise PostgreSQL Company
    >
    
    
    
    -- 
    KaiGai Kohei <kaigai@kaigai.gr.jp>
    

14. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Robert Haas <robertmhaas@gmail.com> — 2011-06-27T17:28:30Z

    On Wed, Jun 22, 2011 at 1:36 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    > On Wed, Jun 22, 2011 at 12:51 PM, Alvaro Herrera
    > <alvherre@commandprompt.com> wrote:
    >> Excerpts from Robert Haas's message of mié jun 22 08:56:02 -0400 2011:
    >>
    >>> Another option might be to leave heap_openrv() and relation_openrv()
    >>> alone and add a missing_ok argument to try_heap_openrv() and
    >>> try_relation_openrv().  Passing true would give the same behavior as
    >>> presently; passing false would make them behave like the non-try
    >>> version.
    >>
    >> That would be pretty weird, having two functions, one of them sometimes
    >> doing the same thing as the other one.
    >>
    >> I understand Noah's concern but I think your original proposal was saner
    >> than both options presented so far.
    >
    > I agree with you.  If we had a whole pile of options it might be worth
    > having heap_openrv() and heap_openrv_extended() so as not to
    > complicate the simple case, but since there's no forseeable need to
    > add anything other than missing_ok, my gut is to just add it and call
    > it good.
    
    On further review, my gut is having second thoughts.  This patch is an
    awful lot smaller and easier to verify correctness if I just mess with
    the "try" calls and not the regular ones; and it avoids both
    back-patching hazards for us and hoops for third-party loadable
    modules that are using the non-try versions of those functions to jump
    through.
    
    Third try attached...
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    

15. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Noah Misch <noah@leadboat.com> — 2011-06-27T18:59:37Z

    On Mon, Jun 27, 2011 at 01:28:30PM -0400, Robert Haas wrote:
    > On Wed, Jun 22, 2011 at 1:36 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    > > I agree with you. ?If we had a whole pile of options it might be worth
    > > having heap_openrv() and heap_openrv_extended() so as not to
    > > complicate the simple case, but since there's no forseeable need to
    > > add anything other than missing_ok, my gut is to just add it and call
    > > it good.
    > 
    > On further review, my gut is having second thoughts.  This patch is an
    > awful lot smaller and easier to verify correctness if I just mess with
    > the "try" calls and not the regular ones; and it avoids both
    > back-patching hazards for us and hoops for third-party loadable
    > modules that are using the non-try versions of those functions to jump
    > through.
    
    +1.  (Note that the function header comments need a few more updates.)
    
    

16. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Robert Haas <robertmhaas@gmail.com> — 2011-06-27T19:57:09Z

    On Mon, Jun 27, 2011 at 2:59 PM, Noah Misch <noah@leadboat.com> wrote:
    > On Mon, Jun 27, 2011 at 01:28:30PM -0400, Robert Haas wrote:
    >> On Wed, Jun 22, 2011 at 1:36 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    >> > I agree with you. ?If we had a whole pile of options it might be worth
    >> > having heap_openrv() and heap_openrv_extended() so as not to
    >> > complicate the simple case, but since there's no forseeable need to
    >> > add anything other than missing_ok, my gut is to just add it and call
    >> > it good.
    >>
    >> On further review, my gut is having second thoughts.  This patch is an
    >> awful lot smaller and easier to verify correctness if I just mess with
    >> the "try" calls and not the regular ones; and it avoids both
    >> back-patching hazards for us and hoops for third-party loadable
    >> modules that are using the non-try versions of those functions to jump
    >> through.
    >
    > +1.  (Note that the function header comments need a few more updates.)
    
    Oh, good catch, thanks.  Committed with some further comment changes.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    

17. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Kohei KaiGai <kaigai@kaigai.gr.jp> — 2011-06-27T20:40:06Z

    The attached patch is rebased one towards the latest tree, using
    relation_openrv_extended().
    
    Although it is not a matter in this patch itself, I found a problem on
    the upcoming patch
    that consolidate routines associated with DropStmt.
    Existing RemoveRelations() acquires a lock on the table owning an
    index to be removed
    in the case when OBJECT_INDEX is supplied.
    However, the revised get_object_address() opens the supplied relation
    (= index) in same
    time with lookup of its name. So, we may break down the
    relation_openrv_extended()
    into a pair of RangeVarGetRelid() and relation_open().
    
    Any good idea?
    
    2011/6/27 Robert Haas <robertmhaas@gmail.com>:
    > On Mon, Jun 27, 2011 at 2:59 PM, Noah Misch <noah@leadboat.com> wrote:
    >> On Mon, Jun 27, 2011 at 01:28:30PM -0400, Robert Haas wrote:
    >>> On Wed, Jun 22, 2011 at 1:36 PM, Robert Haas <robertmhaas@gmail.com> wrote:
    >>> > I agree with you. ?If we had a whole pile of options it might be worth
    >>> > having heap_openrv() and heap_openrv_extended() so as not to
    >>> > complicate the simple case, but since there's no forseeable need to
    >>> > add anything other than missing_ok, my gut is to just add it and call
    >>> > it good.
    >>>
    >>> On further review, my gut is having second thoughts.  This patch is an
    >>> awful lot smaller and easier to verify correctness if I just mess with
    >>> the "try" calls and not the regular ones; and it avoids both
    >>> back-patching hazards for us and hoops for third-party loadable
    >>> modules that are using the non-try versions of those functions to jump
    >>> through.
    >>
    >> +1.  (Note that the function header comments need a few more updates.)
    >
    > Oh, good catch, thanks.  Committed with some further comment changes.
    >
    > --
    > Robert Haas
    > EnterpriseDB: http://www.enterprisedb.com
    > The Enterprise PostgreSQL Company
    >
    
    
    
    -- 
    KaiGai Kohei <kaigai@kaigai.gr.jp>
    

18. Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address

    Robert Haas <robertmhaas@gmail.com> — 2011-06-28T01:24:42Z

    On Mon, Jun 27, 2011 at 4:40 PM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
    > The attached patch is rebased one towards the latest tree, using
    > relation_openrv_extended().
    
    Committed.
    
    > Although it is not a matter in this patch itself, I found a problem on
    > the upcoming patch
    > that consolidate routines associated with DropStmt.
    > Existing RemoveRelations() acquires a lock on the table owning an
    > index to be removed
    > in the case when OBJECT_INDEX is supplied.
    > However, the revised get_object_address() opens the supplied relation
    > (= index) in same
    > time with lookup of its name. So, we may break down the
    > relation_openrv_extended()
    > into a pair of RangeVarGetRelid() and relation_open().
    
    Not without looking at the patch.  I will respond on that thread when
    I've read through it more thoroughly.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company