Thread: 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
Attachment
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.albe@wien.gv.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@cybertec.at> writes:
> Albe Laurenz <laurenz.albe@wien.gv.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@cybertec.at> writes: >> Albe Laurenz <laurenz.albe@wien.gv.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.albe@wien.gv.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@gmail.com> writes:
> On Thu, Nov 20, 2014 at 1:56 PM, Albe Laurenz <laurenz.albe@wien.gv.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@gmail.com> writes: >> On Thu, Nov 20, 2014 at 1:56 PM, Albe Laurenz <laurenz.albe@wien.gv.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.albe@wien.gv.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.albe@wien.gv.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.albe@wien.gv.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'twant to do something here. Especially since we did go down this route with disallowing indexes on timestamptz castedto 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 nothingelse, 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