Thread: Corruption with IMMUTABLE functions in index expression.
Hi All,
While using IMMUTABLE functions in index expression, we are getting below corruption on HEAD.
postgres=# CREATE TABLE tab1 (c1 numeric, c2 numeric);
CREATE TABLE
postgres=# INSERT INTO tab1 values (10, 100);
INSERT 0 1
postgres=# CREATE OR REPLACE FUNCTION func1(var1 numeric)
RETURNS NUMERIC AS $$
DECLARE
result numeric;
BEGIN
SELECT c2 into result FROM tab1 WHERE c1=var1;
RETURN result;
END;
$$ LANGUAGE plpgsql IMMUTABLE;
CREATE FUNCTION
-- When using the IMMUTABLE function in creating an index for the first time, it is working fine.
postgres=# CREATE INDEX idx1 ON tab1(func1(c1));
CREATE INDEX
-- Executing the similar query for 2nd time, We are getting the error
postgres=# CREATE INDEX idx2 ON tab1(func1(c1));
ERROR: could not read block 0 in file "base/13675/16391": read only 0 of 8192 bytes
CONTEXT: SQL statement "SELECT c2 FROM tab1 WHERE c1=var1"
PL/pgSQL function func1(numeric) line 5 at SQL statement
While using IMMUTABLE functions in index expression, we are getting below corruption on HEAD.
postgres=# CREATE TABLE tab1 (c1 numeric, c2 numeric);
CREATE TABLE
postgres=# INSERT INTO tab1 values (10, 100);
INSERT 0 1
postgres=# CREATE OR REPLACE FUNCTION func1(var1 numeric)
RETURNS NUMERIC AS $$
DECLARE
result numeric;
BEGIN
SELECT c2 into result FROM tab1 WHERE c1=var1;
RETURN result;
END;
$$ LANGUAGE plpgsql IMMUTABLE;
CREATE FUNCTION
-- When using the IMMUTABLE function in creating an index for the first time, it is working fine.
postgres=# CREATE INDEX idx1 ON tab1(func1(c1));
CREATE INDEX
-- Executing the similar query for 2nd time, We are getting the error
postgres=# CREATE INDEX idx2 ON tab1(func1(c1));
ERROR: could not read block 0 in file "base/13675/16391": read only 0 of 8192 bytes
CONTEXT: SQL statement "SELECT c2 FROM tab1 WHERE c1=var1"
PL/pgSQL function func1(numeric) line 5 at SQL statement
--
With Regards,
Prabhat Kumar Sahu
EnterpriseDB: http://www.enterprisedb.com
On Monday, October 11, 2021, Prabhat Sahu <prabhat.sahu@enterprisedb.com> wrote:
While using IMMUTABLE functions in index expression, we are getting below corruption on HEAD.
That function is not actually immutable (the system doesn’t check whether your claim of immutability and the function definition match, its up to you to know and specify the correct label for what the function does) so not our problem. Write a trigger instead.
David J.
> 11 окт. 2021 г., в 20:47, David G. Johnston <david.g.johnston@gmail.com> написал(а): > > On Monday, October 11, 2021, Prabhat Sahu <prabhat.sahu@enterprisedb.com> wrote: > While using IMMUTABLE functions in index expression, we are getting below corruption on HEAD. > > That function is not actually immutable (the system doesn’t check whether your claim of immutability and the function definitionmatch, its up to you to know and specify the correct label for what the function does) so not our problem. Writea trigger instead. +1, but the error is strange. This might be a sign of some wrong assumption somewhere. My wild guess is that metapage isread before it was written. Best regards, Andrey Borodin.
On 10/11/21 18:08, Andrey Borodin wrote: > > >> 11 окт. 2021 г., в 20:47, David G. Johnston <david.g.johnston@gmail.com> написал(а): >> >> On Monday, October 11, 2021, Prabhat Sahu <prabhat.sahu@enterprisedb.com> wrote: >> While using IMMUTABLE functions in index expression, we are getting below corruption on HEAD. >> >> That function is not actually immutable (the system doesn’t check whether your claim of immutability and the functiondefinition match, its up to you to know and specify the correct label for what the function does) so not our problem. Write a trigger instead. > +1, but the error is strange. This might be a sign of some wrong assumption somewhere. My wild guess is that metapage isread before it was written. > True, but I can't reproduce it. So either the build is broken in some way, or perhaps there's something else going on. What would be quite helpful is a backtrace showing why the error was triggered. i.e. set a breakpoint on the ereport in mdread(). regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Monday, October 11, 2021, Prabhat Sahu <prabhat.sahu@enterprisedb.com>
> wrote:
>> While using IMMUTABLE functions in index expression, we are getting below
>> corruption on HEAD.
> That function is not actually immutable (the system doesn’t check whether
> your claim of immutability and the function definition match, its up to you
> to know and specify the correct label for what the function does) so not
> our problem. Write a trigger instead.
Yeah. What is happening is that the function's SELECT on the subject
table is trying to examine the not-yet-valid new index. While that could
be argued to be a bug, I share David's lack of interest in fixing it,
because I do not believe that there are any cases where a function that
accesses an index's subject table is really going to be immutable.
To prevent this access, we'd have to set pg_index.indisvalid false
initially and then update it to true after the index is built.
We do do that in CREATE INDEX CONCURRENTLY (so you can make this
example work by specifying CONCURRENTLY), but I'm unexcited about
creating bloat in pg_index for the standard case in order to support
a usage scenario that is going to cause you all sorts of other pain.
To paraphrase Henry Spencer: if you lie to the planner, it will get
its revenge.
regards, tom lane
Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> True, but I can't reproduce it. So either the build is broken in some
> way, or perhaps there's something else going on. What would be quite
> helpful is a backtrace showing why the error was triggered. i.e. set a
> breakpoint on the ereport in mdread().
It reproduced as-described for me. The planner sees the index as
already indisvalid, so it figures it can ask for the tree height:
#0 errfinish (filename=0xa4c15c "md.c", lineno=686,
funcname=0xac6a98 <__func__.13643> "mdread") at elog.c:515
#1 0x00000000004dc8fb in mdread (reln=<optimized out>,
forknum=<optimized out>, blocknum=0, buffer=0x7fad931b4f80 "") at md.c:682
#2 0x00000000007fd15c in ReadBuffer_common (smgr=0x1d72140,
relpersistence=<optimized out>, forkNum=MAIN_FORKNUM, blockNum=0,
mode=RBM_NORMAL, strategy=<optimized out>, hit=0x7fff63a1d7af)
at bufmgr.c:1003
#3 0x00000000007fdb54 in ReadBufferExtended (reln=0x7fad9c4b69d8,
forkNum=MAIN_FORKNUM, blockNum=0, mode=<optimized out>,
strategy=<optimized out>) at ../../../../src/include/utils/rel.h:548
#4 0x00000000005797f5 in _bt_getbuf (rel=0x7fad9c4b69d8,
blkno=<optimized out>, access=1) at nbtpage.c:878
#5 0x0000000000579bc7 in _bt_getrootheight (rel=rel@entry=0x7fad9c4b69d8)
at nbtpage.c:680
#6 0x000000000078106a in get_relation_info (root=root@entry=0x1d84b28,
relationObjectId=59210, inhparent=false, rel=rel@entry=0x1d85290)
at plancat.c:419
#7 0x0000000000785451 in build_simple_rel (root=0x1d84b28, relid=1,
parent=0x0) at relnode.c:308
#8 0x000000000075792f in add_base_rels_to_query (root=root@entry=0x1d84b28,
jtnode=<optimized out>) at initsplan.c:122
#9 0x000000000075ac68 in query_planner (root=root@entry=0x1d84b28,
--Type <RET> for more, q to quit, c to continue without paging--q
regards, tom lane
On Mon, Oct 11, 2021 at 9:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Yeah. What is happening is that the function's SELECT on the subject > table is trying to examine the not-yet-valid new index. While that could > be argued to be a bug, I share David's lack of interest in fixing it, > because I do not believe that there are any cases where a function that > accesses an index's subject table is really going to be immutable. Right. It might be different if this was something that users sometimes expect will work, based on some plausible-though-wrong understanding of expression indexes. But experience suggests that they don't. -- Peter Geoghegan
Hi, On 2021-10-11 12:27:44 -0400, Tom Lane wrote: > While that could be argued to be a bug, I share David's lack of interest in > fixing it, because I do not believe that there are any cases where a > function that accesses an index's subject table is really going to be > immutable. > To prevent this access, we'd have to set pg_index.indisvalid false > initially and then update it to true after the index is built. > We do do that in CREATE INDEX CONCURRENTLY (so you can make this > example work by specifying CONCURRENTLY), but I'm unexcited about > creating bloat in pg_index for the standard case in order to support > a usage scenario that is going to cause you all sorts of other pain. > To paraphrase Henry Spencer: if you lie to the planner, it will get > its revenge. I agree that there's not much point in making this really "work", but perhaps we could try to generate a more useful error message, without incurring undue overhead? I think there've been a few reports of this over the years, including some internally to postgres, e.g. during catalog table index rebuilds. Perhaps we could set pg_index.indisvalid to false initially, and if opening an index where pg_index.indisvalid error out with a different error message if TransactionIdIsCurrentTransactionId(xmin). And then use an inplace update to set indisvalid to true, to avoid the bloat? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> Perhaps we could set pg_index.indisvalid to false initially, and if opening an
> index where pg_index.indisvalid error out with a different error message if
> TransactionIdIsCurrentTransactionId(xmin). And then use an inplace update to
> set indisvalid to true, to avoid the bloat?
I still can't get excited about it ... but yeah, update-in-place would
be enough to remove the bloat objection. I doubt we need any code
changes beyond changing the indisvalid state.
regards, tom lane
Hi, On 2021-10-11 14:59:22 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > Perhaps we could set pg_index.indisvalid to false initially, and if opening an > > index where pg_index.indisvalid error out with a different error message if > > TransactionIdIsCurrentTransactionId(xmin). And then use an inplace update to > > set indisvalid to true, to avoid the bloat? > > I still can't get excited about it ... Understandable, me neither... > I doubt we need any code changes beyond changing the indisvalid state. I was thinking we'd want to throw an error if an index that's being created is accessed during the index build, rather than just not include it in planning... Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2021-10-11 14:59:22 -0400, Tom Lane wrote:
>> I doubt we need any code changes beyond changing the indisvalid state.
> I was thinking we'd want to throw an error if an index that's being created is
> accessed during the index build, rather than just not include it in
> planning...
AFAICT we *will* throw an error, just not a very intelligible one.
But until someone's shown another way to reach that error besides
the planner's path, I'm not thinking we need to expend effort on
making the error nicer.
regards, tom lane