argtype_inherit() is dead code

From: Tom Lane
Subject: argtype_inherit() is dead code
Date: ,
Msg-id: 8924.1113609876@sss.pgh.pa.us
(view: Whole thread, Raw)
Responses: Re: argtype_inherit() is dead code  ( (elein))
List: pgsql-hackers

Tree view

argtype_inherit() is dead code  (Tom Lane, )
 Re: argtype_inherit() is dead code  ( (elein), )
  Re: argtype_inherit() is dead code  (Tom Lane, )
   Re: argtype_inherit() is dead code  (Alvaro Herrera, )
    Re: argtype_inherit() is dead code  (Tom Lane, )
    Re: argtype_inherit() is dead code  ("Joshua D. Drake", )
     Re: argtype_inherit() is dead code  (Alvaro Herrera, )
      Re: argtype_inherit() is dead code  (Rod Taylor, )
       Re: argtype_inherit() is dead code  (Robert Treat, )
        Re: argtype_inherit() is dead code  (Rod Taylor, )
      Re: argtype_inherit() is dead code  (Christopher Kings-Lynne, )
      Re: argtype_inherit() is dead code  (Bruce Momjian, )
      Re: argtype_inherit() is dead code  ("Joshua D. Drake", )
     Re: argtype_inherit() is dead code  ("Jim C. Nasby", )
   Re: argtype_inherit() is dead code  ( (elein), )
 Re: argtype_inherit() is dead code  ("Chuck McDevitt", )
 Re: argtype_inherit() is dead code  (Christopher Browne, )
  Re: argtype_inherit() is dead code  ("Jim C. Nasby", )
 Re: argtype_inherit() is dead code  ("Dave Held", )

In parse_func.c there are routines argtype_inherit() and
gen_cross_product() that date from Berkeley days.  The comments
explain their reason for existence thus:
*  This function is used to handle resolution of function calls when*  there is no match to the given argument types,
butthere might be*  matches based on considering complex types as members of their*  superclass types (parent
classes).** It takes an array of input type ids.  For each type id in the array*  that's a complex type (a class), it
walksup the inheritance tree,*  finding all superclasses of that type. A vector of new Oid type*  arrays is returned to
thecaller, listing possible alternative*  interpretations of the input typeids as members of their superclasses*
ratherthan the actually given argument types. The vector is*  terminated by a NULL pointer.**  The order of this vector
isas follows:  all superclasses of the*  rightmost complex class are explored first.  The exploration*  continues from
rightto left.  This policy means that we favor*  keeping the leftmost argument type as low in the inheritance tree*  as
possible. This is intentional; it is exactly what we need to*  do for method dispatch.**  The vector does not include
thecase where no complex classes have*  been promoted, since that was already tried before this routine*  got called.
 

I realized that this is effectively dead code: although it can be
executed, it can never produce any useful results.  The reason is that
can_coerce_type() already knows that inherited rowtypes can be promoted
to their parent rowtypes, and it considers that a legal implicit
coercion.  This means that any possible function matches based on
promoting child rowtypes to ancestors were found in func_get_detail()'s
first pass.  If there is exactly one match then it will be taken as
the correct answer and returned without calling argtype_inherit().
If there is more than one match then func_get_detail() will fail
(return FUNCDETAIL_MULTIPLE), again without calling argtype_inherit().
The only way to reach argtype_inherit() is if there are *no* ancestor
matches, which means that the function is a very expensive no-op that
we execute just before throwing an error.

I'm strongly tempted to just rip out argtype_inherit() and
gen_cross_product().  Even if we suppose that we might want to resurrect
the claimed functionality someday, I don't think it could be made to
work this way.  You'd have to put the knowledge into
func_select_candidate() instead, else there'd be very weird interactions
with the heuristics for resolving non-complex input argument types.

Thoughts?
        regards, tom lane



pgsql-hackers by date:

From: Tom Lane
Date:
Subject: argtype_inherit() is dead code
From: Bruce Momjian
Date:
Subject: Problem with PITR recovery