Re: lsyscache: free IndexAmRoutine objects returned by GetIndexAmRoutineByAmId()

Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:
> I noticed this problem while reviewing Tom’s patch [1]. In lsyscache.c,
> there are multiple places that get IndexAmRoutine
> by GetIndexAmRoutineByAmId(). Only one function get_opmethod_canorder()
> pfree the returned object, while the other 3
> functions get_op_index_interpretation(), equality_ops_are_compatible()
> and comparison_ops_are_compatible() all call GetIndexAmRoutineByAmId() in
> for loops but without freeing the memory.

> While these paths are not hot enough to cause leaks that matter in
> practice, I think for consistency, we should free the memory.

I wonder if it wouldn't be better to rethink the convention that
IndexAmRoutine structs are palloc'd. Is there any AM for which
the contents aren't constant, so that we could just return a pointer
to a static constant struct and save the palloc/pfree overhead?
We'd want to const-ify the result type of GetIndexAmRoutine, and
maybe that'd result in more notational churn than it's worth,
but it seems like we're missing a bet.

(I would not have proposed this before we started using C99
struct initializers. But now that we can, it doesn't seem
like writing out the struct contents as an initializer would
be too painful.)

regards, tom lane