Thread: Problematic enforcement of "ERROR: functions in index predicate must be marked IMMUTABLE"

Hi,

 

If you attempt to create an index based on function that is not IMMUTABLE you will get an exception "ERROR:  functions in index predicate must be marked IMMUTABLE".  However, if you created the index when the function was IMMUTABLE, but later on you updated the function and mistakenly removed the IMMUTABLE key, you will not get any error to alert you that there is an index based on this function and it should remain IMMUTABLE.

 

I suggest triggering error message also when updating a function that is used by index if it is no longer IMMUTABLE

 

Avi

 

 

 

IMPORTANT - This email and any attachments is intended for the above named addressee(s), and may contain information which is confidential or privileged. If you are not the intended recipient, please inform the sender immediately and delete this email: you should not copy or use this e-mail for any purpose nor disclose its contents to any person.
At Sun, 9 Jul 2023 14:22:37 +0000, Avi Weinberg <AviW@gilat.com> wrote in 
> Hi,
> 
> If you attempt to create an index based on function that is not IMMUTABLE you will get an exception "ERROR:
functionsin index predicate must be marked IMMUTABLE".  However, if you created the index when the function was
IMMUTABLE,but later on you updated the function and mistakenly removed the IMMUTABLE key, you will not get any error to
alertyou that there is an index based on this function and it should remain IMMUTABLE.
 
> 
> I suggest triggering error message also when updating a function that is used by index if it is no longer IMMUTABLE

There's no way to truly verify a function is really immutable or
not. So, as mentioned in the documentation, the function volatility
categories are essentially a promise to the optimizer regarding the
function's behavior.

Even given this, premising users keeping the volatility marks in line
with the actual behavior of their corresponding functions, it might be
benetifical to prohibit changes to the volatility category while it's
being used for indices. In the first place, that protecting indices
from entering an inconsistent state, at least on the surface.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> Even given this, premising users keeping the volatility marks in line
> with the actual behavior of their corresponding functions, it might be
> benetifical to prohibit changes to the volatility category while it's
> being used for indices.

Are you going to prohibit changes to the function's behavior, which is
what actually matters?  And if so, how will you enforce that?  Even if
we had an understanding of the function body --- which we generally
don't for PL functions --- determining that would be equivalent to
solving the halting problem.  "Refuse *all* updates to the pg_proc
entry" might sound like a solution; but it is not, because the
function might call another one whose behavior could get changed.

Even granting that we had a useful way to enforce such a restriction,
figuring out whether to apply it would be subject to race conditions;
maybe somebody else is in process of creating an index using the
function that's being altered.

In the end, adding such restrictions would just give a false sense
of security, because there would always be gaps in whatever we did.
As you quote from the documentation, volatility markings are a promise
by the user to the system, not vice versa.  If you break your promise,
you get to keep both pieces of whatever trouble ensues.

            regards, tom lane



At Tue, 11 Jul 2023 10:14:29 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> At Sun, 9 Jul 2023 14:22:37 +0000, Avi Weinberg <AviW@gilat.com> wrote in 
> > Hi,
> > 
> > If you attempt to create an index based on function that is not IMMUTABLE you will get an exception "ERROR:
functionsin index predicate must be marked IMMUTABLE".  However, if you created the index when the function was
IMMUTABLE,but later on you updated the function and mistakenly removed the IMMUTABLE key, you will not get any error to
alertyou that there is an index based on this function and it should remain IMMUTABLE.
 
> > 
> > I suggest triggering error message also when updating a function that is used by index if it is no longer
IMMUTABLE
> 
> There's no way to truly verify a function is really immutable or
> not. So, as mentioned in the documentation, the function volatility
> categories are essentially a promise to the optimizer regarding the
> function's behavior.
> 
> Even given this, premising users keeping the volatility marks in line
> with the actual behavior of their corresponding functions, it might be
> benetifical to prohibit changes to the volatility category while it's
> being used for indices. In the first place, that protecting indices
> from entering an inconsistent state, at least on the surface.

Even after doing that, any functions used indeirectly are still
allowed to be or to become non-immutable. Therefore, we might want to
reject this because we can't execut it perfectly.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



On Mon, Jul 10, 2023 at 6:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> Even given this, premising users keeping the volatility marks in line
> with the actual behavior of their corresponding functions, it might be
> benetifical to prohibit changes to the volatility category while it's
> being used for indices.

In the end, adding such restrictions would just give a false sense
of security, because there would always be gaps in whatever we did.
As you quote from the documentation, volatility markings are a promise
by the user to the system, not vice versa.  If you break your promise,
you get to keep both pieces of whatever trouble ensues.


I'd accept this more readily if we didn't have user unfriendly behavior for CREATE OR REPLACE FUNCTION.

 postgres=# \df+ immut
                                                                            List of functions
 Schema | Name  | Result data type | Argument data types | Type | Volatility | Parallel |  Owner  | Security | Access privileges | Language | Internal name | Description
--------+-------+------------------+---------------------+------+------------+----------+---------+----------+-------------------+----------+---------------+-------------
 public | immut | text             |                     | func | immutable  | unsafe   | vagrant | invoker  |                   | plpgsql  |               |
(1 row)

postgres=# create or replace function immut() returns text as $$begin select 'one'; end; $$ language plpgsql;
CREATE FUNCTION
postgres=# \df+ immut
                                                                            List of functions
 Schema | Name  | Result data type | Argument data types | Type | Volatility | Parallel |  Owner  | Security | Access privileges | Language | Internal name | Description
--------+-------+------------------+---------------------+------+------------+----------+---------+----------+-------------------+----------+---------------+-------------
 public | immut | text             |                     | func | volatile   | unsafe   | vagrant | invoker  |                   | plpgsql  |               |
(1 row)


I find it quite reasonable to tell the user (warning) that their default choice of volatile violates the immutable clause of the existing function, and even would go so far as to require them to drop/recreate the function if indeed their goal is to change it from immutable to volatile (error).  To the extreme I'd just add "changing the volatility marker" to be prohibited just like we prohibit changing the return type.

David J.

At Mon, 10 Jul 2023 21:28:12 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
> In the end, adding such restrictions would just give a false sense
> of security, because there would always be gaps in whatever we did.
> As you quote from the documentation, volatility markings are a promise
> by the user to the system, not vice versa.  If you break your promise,
> you get to keep both pieces of whatever trouble ensues.

I agree to you, as I mentioned a-bit-too-late message..

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center