Re: Performance problem in textanycat/anytextcat

On Mon, May 17, 2010 at 4:01 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>>> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>>>> Marking textanycat as not immutable would forbid using it in
>>>> expression indexes, too.
>
>>> True.  On the other hand, the current state of affairs allows one to
>>> create an index on expressions that aren't really immutable, with
>>> ensuing hilarity.
>
>> It strikes me that we could avoid any possible functional regression
>> here by having CREATE INDEX perform expression preprocessing (in
>> particular, function inlining) before it tests to see if the index
>> expression contains any non-immutable functions.
>
> I looked into this and found that it's a pretty trivial change to make
> CREATE INDEX do it that way.  Furthermore, this will cover us against
> a subtle gotcha that was introduced by the addition of default arguments
> for functions: what happens if a marked-as-immutable function has a
> volatile default argument?  I don't think that's an insane combination,
> since the function's own processing might be perfectly immutable.  But
> right now, CREATE INDEX will draw the wrong conclusion about whether a
> function call that uses the default is safe to put into an index.
>
> BTW, I looked around for other places where we might be making the same
> mistake.  AFAICT, these two checks in CREATE INDEX are the only places
> outside the planner that use either contain_mutable_functions or
> contain_volatile_functions.  The ones inside-the-planner are OK because
> they are looking at already-preprocessed expressions.
>
> Perhaps this is a backpatchable bug fix.  Comments?

I can't say whether this is safe enough to back-patch, but the way
this is set up, don't we also need to fix some catalog entries and, if
yes, isn't that problematic?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company