Re: Functions used in index definitions shouldn't be changed

Currently it is possible to change the behaviour of a function with
CREATE OR REPLACE FUNCTION even if the function is part of an index definition.

I think that should be forbidden, because it is very likely to corrupt
the index. I expect the objection that this would break valid use cases
where people know exactly what they are doing, but I believe that this
is a footgun for inexperienced users that should be disarmed.

I'd also opt for forbidding behaviour changing modifications with ALTER FUNCTION
for functions used in index definitions, specifically altering strictness.

Attached is a patch implementing a fix.

Yours,
Laurenz Albe

On 11/19/14 3:38 PM, Albe Laurenz wrote:
> I think that should be forbidden, because it is very likely to corrupt
> the index. I expect the objection that this would break valid use cases
> where people know exactly what they are doing, but I believe that this
> is a footgun for inexperienced users that should be disarmed.

Yes, I believe that objection to be completely valid. I can't begin to
describe how frustrating it is to be in the position where my hammer is
made out of plastic because someone might hurt themself if it was a real
one.

I'd at least like to see a workaround for the "less inexperienced" users.

.marko

Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
> Currently it is possible to change the behaviour of a function with
> CREATE OR REPLACE FUNCTION even if the function is part of an index definition.
>
> I think that should be forbidden, because it is very likely to corrupt
> the index. I expect the objection that this would break valid use cases
> where people know exactly what they are doing, but I believe that this
> is a footgun for inexperienced users that should be disarmed.
>
> I'd also opt for forbidding behaviour changing modifications with ALTER FUNCTION
> for functions used in index definitions, specifically altering strictness.
>
> Attached is a patch implementing a fix.

Instead of adding extra check, shouldn't you just ensure that
DEPENDENCY_INTERNAL is the dependency type and let the existing logic do the work?

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at

Antonin Houska <ah(at)cybertec(dot)at> writes:
> Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
>> Currently it is possible to change the behaviour of a function with
>> CREATE OR REPLACE FUNCTION even if the function is part of an index definition.
>>
>> I think that should be forbidden, because it is very likely to corrupt
>> the index. I expect the objection that this would break valid use cases
>> where people know exactly what they are doing, but I believe that this
>> is a footgun for inexperienced users that should be disarmed.
>>
>> I'd also opt for forbidding behaviour changing modifications with ALTER FUNCTION
>> for functions used in index definitions, specifically altering strictness.
>>
>> Attached is a patch implementing a fix.

> Instead of adding extra check, shouldn't you just ensure that
> DEPENDENCY_INTERNAL is the dependency type and let the existing logic do the work?

The dependency logic only prohibits DROP, not ALTER, so changing the
dependency type wouldn't be enough to accomplish that. But I dislike the
entire concept, so implementation details are not that interesting.

A comparable issue is that alteration of text search configurations may
partially invalidate precomputed tsvector columns and indexes based on
tsvector data. We discussed whether we should prohibit that somehow
and explicitly decided that it would be a bad idea. In the text search
case, changes like adding stop words are common and don't meaningfully
invalidate indexes. In some cases you may decide that you need to
recompute the tsvector data, but that decision should be left to the
user.

I think that pretty much the same thing applies to functions used in
indexes. There are too many use-cases where alterations don't
meaningfully impact the stored data for us to get away with a blanket
prohibition. (I'm pretty sure that this has been discussed in the
past, too. If Albe really wants to push the point he should look
up the previous discussions and explain why the decision was wrong.)

regards, tom lane

Tom Lane wrote:
> Antonin Houska <ah(at)cybertec(dot)at> writes:
>> Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
>>> Currently it is possible to change the behaviour of a function with
>>> CREATE OR REPLACE FUNCTION even if the function is part of an index definition.
>>>
>>> I think that should be forbidden, because it is very likely to corrupt
>>> the index. I expect the objection that this would break valid use cases
>>> where people know exactly what they are doing, but I believe that this
>>> is a footgun for inexperienced users that should be disarmed.
>>>
>>> I'd also opt for forbidding behaviour changing modifications with ALTER FUNCTION
>>> for functions used in index definitions, specifically altering strictness.
>>>
>>> Attached is a patch implementing a fix.

> A comparable issue is that alteration of text search configurations may
> partially invalidate precomputed tsvector columns and indexes based on
> tsvector data. We discussed whether we should prohibit that somehow
> and explicitly decided that it would be a bad idea. In the text search
> case, changes like adding stop words are common and don't meaningfully
> invalidate indexes. In some cases you may decide that you need to
> recompute the tsvector data, but that decision should be left to the
> user.
>
> I think that pretty much the same thing applies to functions used in
> indexes. There are too many use-cases where alterations don't
> meaningfully impact the stored data for us to get away with a blanket
> prohibition. (I'm pretty sure that this has been discussed in the
> past, too. If Albe really wants to push the point he should look
> up the previous discussions and explain why the decision was wrong.)

I don't think that there is a universally compelling right or wrong to
questions like this, it is more a matter of taste. Is it more important to protect
the casual DBA from hurting himself or herself, or is it more important to
provide a well honed scalpel for the experienced surgeon?

It seems that more people lean in the latter direction, so I'll cease and desist.

Yours,
Laurenz Albe

On Thu, Nov 20, 2014 at 1:56 PM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
> I don't think that there is a universally compelling right or wrong to
> questions like this, it is more a matter of taste. Is it more important to protect
> the casual DBA from hurting himself or herself, or is it more important to
> provide a well honed scalpel for the experienced surgeon?

+1.

I think if we had an already-existing prohibition here and you
proposed relaxing it, the howls would be equally loud. We're not
entirely consistent about how picky we are.

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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Nov 20, 2014 at 1:56 PM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
>> I don't think that there is a universally compelling right or wrong to
>> questions like this, it is more a matter of taste. Is it more important to protect
>> the casual DBA from hurting himself or herself, or is it more important to
>> provide a well honed scalpel for the experienced surgeon?

> +1.

> I think if we had an already-existing prohibition here and you
> proposed relaxing it, the howls would be equally loud. We're not
> entirely consistent about how picky we are.

How's that quote about foolish consistency go? In many cases, the reason
why we enforce some things and not others is practical utility.

regards, tom lane

On 11/20/2014 02:28 PM, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Thu, Nov 20, 2014 at 1:56 PM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
>>> I don't think that there is a universally compelling right or wrong to
>>> questions like this, it is more a matter of taste. Is it more important to protect
>>> the casual DBA from hurting himself or herself, or is it more important to
>>> provide a well honed scalpel for the experienced surgeon?
>> +1.
>> I think if we had an already-existing prohibition here and you
>> proposed relaxing it, the howls would be equally loud. We're not
>> entirely consistent about how picky we are.
> How's that quote about foolish consistency go? In many cases, the reason
> why we enforce some things and not others is practical utility.

Right.

(FTR, the quote from Emerson goes "A foolish consistency is the
hobgoblin of little minds, adored by little statesmen and philosophers
and divines.")

cheers

andrew

Robert Haas wrote:
> On Thu, Nov 20, 2014 at 1:56 PM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
> > I don't think that there is a universally compelling right or wrong to
> > questions like this, it is more a matter of taste. Is it more important to protect
> > the casual DBA from hurting himself or herself, or is it more important to
> > provide a well honed scalpel for the experienced surgeon?
>
> +1.
>
> I think if we had an already-existing prohibition here and you
> proposed relaxing it, the howls would be equally loud. We're not
> entirely consistent about how picky we are.

There is also the possibility to add syntax like this:

CREATE OR REPLACE [FORCE] FUNCTION ...

What do you think about that? It would protect the casual user but allow
the expert to do it anyway.

Another thing I thought about is changing function volatility:
If you change the volatility of a function used in an index to anything
other than IMMUTABLE, your database will continue to work as expected, but
a dump will fail to restore with
ERROR: functions in index expression must be marked IMMUTABLE

Is that something worth checking for?

Yours,
Laurenz Albe

Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> writes:
> There is also the possibility to add syntax like this:
> CREATE OR REPLACE [FORCE] FUNCTION ...
> What do you think about that? It would protect the casual user but allow
> the expert to do it anyway.

I don't see any great attraction to that.

regards, tom lane

On 11/21/14, 9:06 AM, Tom Lane wrote:
> Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> writes:
>> There is also the possibility to add syntax like this:
>> CREATE OR REPLACE [FORCE] FUNCTION ...
>> What do you think about that? It would protect the casual user but allow
>> the expert to do it anyway.
>
> I don't see any great attraction to that.

Given what this would do to someone's data, and our general stance on not hurting data, I'm a bit surprised that we don't want to do something here. Especially since we did go down this route with disallowing indexes on timestamptz casted to date, which seriously impacts a lot of reporting scenarios.

I fully agree that it's impractical to completely restrict this case, but something akin to FORCE seems reasonable. If nothing else, I'd think we should at least issue a warning if someone does something that might affect index viability.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com