pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedAW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
19+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2023-10-26 178ee1d858 01575ad788 Prevent duplicate RTEPermissionInfo for plain-inheritance parents
2023-10-24 df73ca3514 be2502947f cf89d3052d 74e5ea1e00 Doc: indexUnchanged is strictly a hint.
* AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
@ 2023-10-23 17:05 Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Hans Buschmann @ 2023-10-23 17:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hello all,
I finally managed to reproduce the bug on a demo database as shown below.
The file err_demo.sql shows all the steps to reproduce. The table creation is done with errdb_noerr_231008.sql (unchanged from last mail).
The clue is:
- you have an inherited table
- on the archiv part, you have only (one or more) brin indexes
- to reorder the tuples before moving to a new major version (with pg_dumpo/restore) the archiv part is clustered.
this requires to create a btree index, which is removed immediately after the cluster statement.
when all this has been done, a write access to the archiv-part of the table hierarchy causes the error.
A direct update access to or_followup_archiv clears the error (as shown in the final comment part of err_demo.sql).
To make the error reappear, you have to redo the 3 recluster steps.
PS: please apply the steps of err_demo.sql manually because it was not tested for full automation.
Hope this enables you to investigate the root cause of the error.
Hans Buschmann
________________________________
Attachments:
err_demo.sql (application/octet-stream, 3.2K) download
errdb_noerr_231008.sql (application/octet-stream, 4.3K) download
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
@ 2023-10-23 18:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 19+ messages in thread
From: Tom Lane @ 2023-10-23 18:30 UTC (permalink / raw)
To: Hans Buschmann <buschmann@nidsa.net>; +Cc: Amit Langote <amitlangote09@gmail.com>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Hans Buschmann <buschmann@nidsa.net> writes:
> I finally managed to reproduce the bug on a demo database as shown below.
Thanks! Reproduced it as described on HEAD (although for me, repeating
the query repeats the error, there's no need to do anything fancy to
re-arm it). Backtrace looks like
#0 errfinish (filename=0xaad704 "parse_relation.c", lineno=3907,
funcname=0xaae690 <__func__.24632> "getRTEPermissionInfo") at elog.c:480
#1 0x00000000004b07fa in getRTEPermissionInfo (rteperminfos=<optimized out>,
rte=0x1725ba8) at parse_relation.c:3906
#2 0x00000000006d6345 in GetResultRTEPermissionInfo (
estate=estate@entry=0x1739120, relinfo=<optimized out>,
relinfo=<optimized out>) at execUtils.c:1386
#3 0x00000000006d755c in ExecGetUpdatedCols (relinfo=relinfo@entry=0x17395e0,
estate=estate@entry=0x1739120) at execUtils.c:1295
#4 0x00000000006c8292 in index_unchanged_by_update (indexRelation=0x1772770,
indexInfo=0x17ea4a0, estate=0x1739120, resultRelInfo=0x17395e0)
at execIndexing.c:985
#5 ExecInsertIndexTuples (resultRelInfo=resultRelInfo@entry=0x17395e0,
slot=slot@entry=0x17ea140, estate=0x1739120, update=update@entry=true,
noDupErr=noDupErr@entry=false, specConflict=specConflict@entry=0x0,
arbiterIndexes=0x0, onlySummarizing=false) at execIndexing.c:427
#6 0x00000000006f560a in ExecUpdateEpilogue (
resultRelInfo=resultRelInfo@entry=0x17395e0,
tupleid=tupleid@entry=0x7ffd1fa2b10e, oldtuple=oldtuple@entry=0x0,
slot=0x17ea140, updateCxt=<optimized out>, context=<optimized out>,
context=<optimized out>) at nodeModifyTable.c:2130
#7 0x00000000006f6b24 in ExecUpdate (context=0x7ffd1fa2b140,
resultRelInfo=0x17395e0, tupleid=0x7ffd1fa2b10e, oldtuple=0x0,
slot=0x17ea140, canSetTag=<optimized out>) at nodeModifyTable.c:2478
#8 0x00000000006f834b in ExecModifyTable (pstate=<optimized out>)
at nodeModifyTable.c:3824
In ExecGetUpdatedCols,
(gdb) p relinfo->ri_RootResultRelInfo
$2 = (struct ResultRelInfo *) 0x0
(gdb) p relinfo->ri_RangeTableIndex
$3 = 5
RTE 5 is the added-on RTE for the child table or_followup_archiv.
It looks like
{RANGETBLENTRY
:alias
{ALIAS
:aliasname or_followup
:colnames ("id_of" "of_season" "of_pr1_fac_netto" "of_pr1_fac_netusd"
"of_pr2_fac_netto" "of_pr2_fac_netusd" "of_pr1_our_netto" "of_pr1_ou
r_netusd" "of_pr2_our_netto" "of_pr2_our_netusd")
}
:eref
{ALIAS
:aliasname or_followup
:colnames ("id_of" "of_season" "of_pr1_fac_netto" "of_pr1_fac_netusd"
"of_pr2_fac_netto" "of_pr2_fac_netusd" "of_pr1_our_netto" "of_pr1_ou
r_netusd" "of_pr2_our_netto" "of_pr2_our_netusd")
}
:rtekind 0
:relid 41352 or_followup_archiv
:relkind r
:rellockmode 3
:tablesample <>
:perminfoindex 0
:lateral false
:inh false
:inFromCl false
:securityQuals <>
}
So the proximate problem is that we're trying to fetch an updatedCols
bitmapset for a child table that has no permissions checks to make
and thus doesn't need an RTEPermissionInfo, so its perminfoindex is
zero. It might be that this relinfo should have ri_RootResultRelInfo
set, but it doesn't. It's also possible that we erred by taking those
bitmapsets out of RTEs, although that'd be an unpleasant conclusion
with v16 already released.
However ... the need to fetch that data is ultimately coming from
index_unchanged_by_update, and I don't see how that is not buggy
as can be, independently of this. How can it be okay to ignore
the effects of BEFORE triggers when deciding if an index is unchanged?
If this is because it's "only a hint" and doesn't have to be reliable,
okay, but the documentation around indexUnchanged utterly fails to
make that clear. I fear some poor index AM writer is going to get
screwed big time when they assume this flag is good for more than
heuristic decisions about when to do noncritical maintenance.
The reason I'm on about that is that if it's okay for
index_unchanged_by_update to lie, another approach we could consider
is to return a default result if we're looking at a child table
for which we lack updatedCols data. We might have to do that in
v16, since we don't have much wiggle room to adjust the
RTEPermissionInfo data in a released branch.
(I haven't looked into exactly why it's so hard to reach this
error. Seems like any inherited UPDATE is at risk, so there
must be additional gating factors to explain why we've not
noticed this before.)
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-23 18:49 ` Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 19+ messages in thread
From: Peter Geoghegan @ 2023-10-23 18:49 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Oct 23, 2023 at 11:30 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> However ... the need to fetch that data is ultimately coming from
> index_unchanged_by_update, and I don't see how that is not buggy
> as can be, independently of this. How can it be okay to ignore
> the effects of BEFORE triggers when deciding if an index is unchanged?
> If this is because it's "only a hint" and doesn't have to be reliable,
> okay, but the documentation around indexUnchanged utterly fails to
> make that clear. I fear some poor index AM writer is going to get
> screwed big time when they assume this flag is good for more than
> heuristic decisions about when to do noncritical maintenance.
That's fair, though note that index_unchanged_by_update does at least
own the fact that it ignores the effects of BEFORE triggers in code
comments. It also doesn't care about predicates in partial indexes,
for reasons that are fairly specific to the way that the hint is
actually used on the nbtree side.
> The reason I'm on about that is that if it's okay for
> index_unchanged_by_update to lie, another approach we could consider
> is to return a default result if we're looking at a child table
> for which we lack updatedCols data. We might have to do that in
> v16, since we don't have much wiggle room to adjust the
> RTEPermissionInfo data in a released branch.
Note that index_unchanged_by_update already lies on v14: it
unconditionally indicates that any non-HOT update has unchanged
indexes for all indexes, regardless of their individual status. This
was due to a pragmatic trade-off made when looking for a backpatchable
fix, but I don't think that there's actually much downside to it. In
general I suspect that index_unchanged_by_update is a little
overengineered.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
@ 2023-10-23 18:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-23 18:55 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Peter Geoghegan <pg@bowt.ie> writes:
> On Mon, Oct 23, 2023 at 11:30 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> If this is because it's "only a hint" and doesn't have to be reliable,
>> okay, but the documentation around indexUnchanged utterly fails to
>> make that clear. I fear some poor index AM writer is going to get
>> screwed big time when they assume this flag is good for more than
>> heuristic decisions about when to do noncritical maintenance.
> That's fair, though note that index_unchanged_by_update does at least
> own the fact that it ignores the effects of BEFORE triggers in code
> comments. It also doesn't care about predicates in partial indexes,
> for reasons that are fairly specific to the way that the hint is
> actually used on the nbtree side.
Yeah, there are comments within index_unchanged_by_update about those
things. What I'm unhappy about is that indexam.sgml's discussion of
the indexUnchanged flag makes it sound far more trustworthy than it
actually is. Somebody who just read that doco and didn't scour the
underlying code would be badly misled.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-23 19:01 ` Peter Geoghegan <pg@bowt.ie>
2023-10-23 22:15 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 19+ messages in thread
From: Peter Geoghegan @ 2023-10-23 19:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Oct 23, 2023 at 11:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah, there are comments within index_unchanged_by_update about those
> things. What I'm unhappy about is that indexam.sgml's discussion of
> the indexUnchanged flag makes it sound far more trustworthy than it
> actually is. Somebody who just read that doco and didn't scour the
> underlying code would be badly misled.
I understand. I'll come up with a doc patch for that later on today.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
@ 2023-10-23 22:15 ` Peter Geoghegan <pg@bowt.ie>
2023-10-24 00:28 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Peter Geoghegan @ 2023-10-23 22:15 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Oct 23, 2023 at 12:01 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Mon, Oct 23, 2023 at 11:55 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Yeah, there are comments within index_unchanged_by_update about those
> > things. What I'm unhappy about is that indexam.sgml's discussion of
> > the indexUnchanged flag makes it sound far more trustworthy than it
> > actually is. Somebody who just read that doco and didn't scour the
> > underlying code would be badly misled.
>
> I understand. I'll come up with a doc patch for that later on today.
What do you think of the attached?
If there are no objections I'll commit this soon, backpatching to v14.
Separately, I wonder if index_unchanged_by_update should actually just
always give the hint with a non-HOT update, regardless of the
specifics for each index/its columns -- just like on the v14 branch.
In general I'm more concerned about the danger of not giving the hint
when we should, rather than giving the hint too often. Most individual
btinsert() calls do nothing with the hint already (because there is
still enough free space for the incoming item on the page). The hint
is inherently nothing more than a signal that bottom-up index deletion
might be a good idea, iff we're just about out of better options for
affected leaf pages.
--
Peter Geoghegan
Attachments:
v1-0001-Doc-indexUnchanged-is-strictly-a-hint.patch (application/octet-stream, 1.4K) download
From 80bf0f3fc8e894c5f2e24728ca6aa75a0de665d7 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Mon, 23 Oct 2023 15:00:42 -0700
Subject: [PATCH v1] Doc: indexUnchanged is strictly a hint.
---
doc/src/sgml/indexam.sgml | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index e813e2b62..30eda37af 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -332,9 +332,13 @@ aminsert (Relation indexRelation,
modify any columns covered by the index, but nevertheless requires a
new version in the index. The index AM may use this hint to decide
to apply bottom-up index deletion in parts of the index where many
- versions of the same logical row accumulate. Note that updating a
- non-key column does not affect the value of
- <literal>indexUnchanged</literal>.
+ versions of the same logical row accumulate. Note that updating a non-key
+ column or a column that only appears in a partial index predicate does not
+ affect the value of <literal>indexUnchanged</literal>. The core code
+ determines each tuple's <literal>indexUnchanged</literal> value using a low
+ overhead approach that allows both false positives and false negatives.
+ Index AMs must not treat <literal>indexUnchanged</literal> as an
+ authoritative source of information about tuple visibility or versioning.
</para>
<para>
--
2.42.0
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 22:15 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
@ 2023-10-24 00:28 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 00:45 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-24 00:28 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Peter Geoghegan <pg@bowt.ie> writes:
> On Mon, Oct 23, 2023 at 12:01 PM Peter Geoghegan <pg@bowt.ie> wrote:
>> I understand. I'll come up with a doc patch for that later on today.
> What do you think of the attached?
WFM.
> Separately, I wonder if index_unchanged_by_update should actually just
> always give the hint with a non-HOT update, regardless of the
> specifics for each index/its columns -- just like on the v14 branch.
I'm confused. Wouldn't that be the exact opposite of "unchanged"?
Maybe the real problem here is that the meaning of the hint is not
what you'd expect from its name?
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 22:15 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-24 00:28 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-24 00:45 ` Peter Geoghegan <pg@bowt.ie>
2023-10-24 01:06 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Peter Geoghegan @ 2023-10-24 00:45 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Oct 23, 2023 at 5:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Separately, I wonder if index_unchanged_by_update should actually just
> > always give the hint with a non-HOT update, regardless of the
> > specifics for each index/its columns -- just like on the v14 branch.
>
> I'm confused. Wouldn't that be the exact opposite of "unchanged"?
Well, in practice "indexUnchanged = true" means "do bottom-up deletion
if it's the only way to avoid a page split". The justification is that
the incoming tuple is "logically unchanged" (actually it's more
complicated than that, but that's our starting point). Maybe that
naming convention makes things more confusing than necessary. Naming
things is hard.
> Maybe the real problem here is that the meaning of the hint is not
> what you'd expect from its name?
Maybe. Perhaps I should have chosen a name that made it clearer that
there really is only one way to apply "indexUnchanged = true". Though
the docs are pretty clear about this already. I can't say I feel too
strongly about the name myself.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 22:15 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-24 00:28 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 00:45 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
@ 2023-10-24 01:06 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 01:23 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-24 01:06 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Peter Geoghegan <pg@bowt.ie> writes:
> On Mon, Oct 23, 2023 at 5:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Separately, I wonder if index_unchanged_by_update should actually just
>>> always give the hint with a non-HOT update, regardless of the
>>> specifics for each index/its columns -- just like on the v14 branch.
>> I'm confused. Wouldn't that be the exact opposite of "unchanged"?
> Well, in practice "indexUnchanged = true" means "do bottom-up deletion
> if it's the only way to avoid a page split". The justification is that
> the incoming tuple is "logically unchanged" (actually it's more
> complicated than that, but that's our starting point).
But doesn't the need for a non-HOT update show that the tuple *was*
changed --- in index-relevant columns, even? Maybe I'm still not
understanding exactly what condition we're detecting.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-23 22:15 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-24 00:28 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 00:45 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Peter Geoghegan <pg@bowt.ie>
2023-10-24 01:06 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-24 01:23 ` Peter Geoghegan <pg@bowt.ie>
0 siblings, 0 replies; 19+ messages in thread
From: Peter Geoghegan @ 2023-10-24 01:23 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Amit Langote <amitlangote09@gmail.com>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Oct 23, 2023 at 6:06 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Well, in practice "indexUnchanged = true" means "do bottom-up deletion
> > if it's the only way to avoid a page split". The justification is that
> > the incoming tuple is "logically unchanged" (actually it's more
> > complicated than that, but that's our starting point).
>
> But doesn't the need for a non-HOT update show that the tuple *was*
> changed --- in index-relevant columns, even? Maybe I'm still not
> understanding exactly what condition we're detecting.
Not necessarily. For one thing you might need to do a non-HOT update
purely because there isn't enough free space to fit the successor
version on the original heap page. In general, even a 100% HOT-safe
UPDATE statement might not be able to perform HOT updates.
As I said earlier, the way that we deal with partial indexes doesn't
really make too much sense if you try to shoehorn it into an abstract
definition. It makes a lot more sense when seen in the context of a
workload with a partial index, and shown by a test case from Marko
Tiikkaja:
https://www.postgresql.org/message-id/flat/CAL9smLC%3DSxYiN7yZ4HDyk0RnZyXoP2vaHD-Vg1JskOEHyhMXug%40m...
I freely admit that this general approach is non-modular, even ugly.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-23 20:59 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
1 sibling, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-23 20:59 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
I wrote:
> So the proximate problem is that we're trying to fetch an updatedCols
> bitmapset for a child table that has no permissions checks to make
> and thus doesn't need an RTEPermissionInfo, so its perminfoindex is
> zero. It might be that this relinfo should have ri_RootResultRelInfo
> set, but it doesn't.
That seems to be a fairly good guess. I was able to reduce the
test case to this:
drop table if exists t1 cascade;
create table t1 (f1 int, f2 int, f3 int
, check (f1 < 10) no inherit
);
create table t2 () inherits(t1);
insert into t2 select i, i+1, 0 from generate_series(1,1000) i;
create index on t2(f1,f2);
update t1 set f3 = 11 where f1 = 12 and f2 = 13;
You don't see the failure without the check constraint on t1.
What is happening is that with that, we remove t1 from the plan
entirely, causing us to think that t2 is the root target relation,
and then things blow up because the root should have an
RTEPermissionInfo and doesn't. The place where things start
to go off the rails is in ExecInitModifyTable:
* If it's a partitioned table, the root partition doesn't appear
* elsewhere in the plan and its RT index is given explicitly in
* node->rootRelation. Otherwise (i.e. table inheritance) the target
* relation is the first relation in the node->resultRelations list.
node->resultRelations contains only t2, so boom.
Maybe we could fix this by setting ModifyTable.rootRelation to
the parent relation in the plain-inheritance case not only the
partitioned case; but I've not investigated what else might be
expecting it to be set only for partitions.
According to this understanding, the root cause is quite old
and the RTEPermissionInfo failure just exposes it in an obvious
way. I wonder whether we can find related cases that misbehave
even before v16.
> (I haven't looked into exactly why it's so hard to reach this
> error. Seems like any inherited UPDATE is at risk, so there
> must be additional gating factors to explain why we've not
> noticed this before.)
I haven't tracked that down yet, but I did find that without the
CHECK constraint we don't reach index_unchanged_by_update at
all in repeat executions of the problem query. This is because
ExecUpdateEpilogue sees updateCxt->updateIndexes == TU_None
and thinks it doesn't have to do anything. This seems a bit
suspicious in its own right. Maybe it's okay, because the
updates would be HOT in this test case, but the first one
should be HOT too. It smells like something is being cached
across queries that probably should not be.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-24 09:48 ` Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Amit Langote @ 2023-10-24 09:48 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Tue, Oct 24, 2023 at 5:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > So the proximate problem is that we're trying to fetch an updatedCols
> > bitmapset for a child table that has no permissions checks to make
> > and thus doesn't need an RTEPermissionInfo, so its perminfoindex is
> > zero. It might be that this relinfo should have ri_RootResultRelInfo
> > set, but it doesn't.
>
> That seems to be a fairly good guess. I was able to reduce the
> test case to this:
>
> drop table if exists t1 cascade;
>
> create table t1 (f1 int, f2 int, f3 int
> , check (f1 < 10) no inherit
> );
>
> create table t2 () inherits(t1);
>
> insert into t2 select i, i+1, 0 from generate_series(1,1000) i;
>
> create index on t2(f1,f2);
>
> update t1 set f3 = 11 where f1 = 12 and f2 = 13;
>
>
> You don't see the failure without the check constraint on t1.
> What is happening is that with that, we remove t1 from the plan
> entirely, causing us to think that t2 is the root target relation,
> and then things blow up because the root should have an
> RTEPermissionInfo and doesn't. The place where things start
> to go off the rails is in ExecInitModifyTable:
>
> * If it's a partitioned table, the root partition doesn't appear
> * elsewhere in the plan and its RT index is given explicitly in
> * node->rootRelation. Otherwise (i.e. table inheritance) the target
> * relation is the first relation in the node->resultRelations list.
>
> node->resultRelations contains only t2, so boom.
>
> Maybe we could fix this by setting ModifyTable.rootRelation to
> the parent relation in the plain-inheritance case not only the
> partitioned case; but I've not investigated what else might be
> expecting it to be set only for partitions.
That appears to be the easiest fix to me (attached PoC makes the error
go away for me and passes check-world).
By setting rootRelation for plan-inheritance roots, there would be two
ResultRelInfos for the root relation (that is, if it's also present in
resultRelations, unlike in this case) -- one in
ModifyTableState.rootResultRelInfo and another in the 1st element of
ModifyTableState.resultRelInfo[]. That might be fine, because,
AFAICS, the partitioning-related code sites that look at the former
have checks to ensure that they don't accidentally try to access a
plain-inheritance relation as a partitioned one. I might need to take
a closer look.
> According to this understanding, the root cause is quite old
> and the RTEPermissionInfo failure just exposes it in an obvious
> way. I wonder whether we can find related cases that misbehave
> even before v16.
I decided to look at fire{BS|AS}Triggers() first, because we changed
them in f49b85d783f to look at ModifyTableState.rootResultRelInfo to
get the table whose triggers to fire. They seem broken for this case:
create or replace function t1_stmt_trig_func() returns trigger as $$
begin raise notice 'updating t1'; return NULL; end; $$ language
plpgsql;
create trigger t1_stmt_trig before update on t1 execute function
t1_stmt_trig_func();
-- trigger should've been fired
update t1 set f3 = 11 where f1 = 12 and f2 = 13;
ERROR: invalid perminfoindex 0 in RTE with relid 32799
-- partitioned roots seem fine
create table p (a int check (a = 0)) partition by list (a);
create table p1 partition of p default;
create trigger p_stmt_trig before update on p execute function
t1_stmt_trig_func();
update p set a = 1 where a = 1;
NOTICE: updating t1
UPDATE 0
I recalled we fixed a similar case in ab5fcf2b04f. I suspect we would
not have seen this report if we had also considered the
excluded-root-parent case in that commit, though I might be wrong.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
Attachments:
set-RootRelation-plain-inheritance.diff (application/octet-stream, 1.3K) download
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index aafef24cf1..c711888073 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1792,6 +1792,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
List *returningLists = NIL;
List *mergeActionLists = NIL;
List *rowMarks;
+ bool inherited_update = false;
if (bms_membership(root->all_result_relids) == BMS_MULTIPLE)
{
@@ -1800,6 +1801,8 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
parse->resultRelation);
int resultRelation = -1;
+ inherited_update = true;
+
/* Add only leaf children to ModifyTable. */
while ((resultRelation = bms_next_member(root->leaf_result_relids,
resultRelation)) >= 0)
@@ -1938,7 +1941,8 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
* If target is a partition root table, we need to mark the
* ModifyTable node appropriately for that.
*/
- if (rt_fetch(parse->resultRelation, parse->rtable)->relkind ==
+ if (inherited_update ||
+ rt_fetch(parse->resultRelation, parse->rtable)->relkind ==
RELKIND_PARTITIONED_TABLE)
rootRelation = parse->resultRelation;
else
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
@ 2023-10-24 14:48 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-24 14:48 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Amit Langote <amitlangote09@gmail.com> writes:
> On Tue, Oct 24, 2023 at 5:59 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Maybe we could fix this by setting ModifyTable.rootRelation to
>> the parent relation in the plain-inheritance case not only the
>> partitioned case; but I've not investigated what else might be
>> expecting it to be set only for partitions.
> That appears to be the easiest fix to me (attached PoC makes the error
> go away for me and passes check-world).
> By setting rootRelation for plan-inheritance roots, there would be two
> ResultRelInfos for the root relation (that is, if it's also present in
> resultRelations, unlike in this case) -- one in
> ModifyTableState.rootResultRelInfo and another in the 1st element of
> ModifyTableState.resultRelInfo[]. That might be fine, because,
> AFAICS, the partitioning-related code sites that look at the former
> have checks to ensure that they don't accidentally try to access a
> plain-inheritance relation as a partitioned one. I might need to take
> a closer look.
I suggest that it might be cleaner if we make rootRelation point
to the original appendrel (that is, the RTE entry with inh = true).
That would be exactly parallel to the partitioning situation, in
that that RTE is not scanned in the plan. But it's for the same
table as what's normally the first result table, so it should have
the same permissions info.
BTW, I noted while looking at this example that there are two
RTEPermissionInfo structs with identical contents, one for the
appendrel RTE and one for the first child. That seems kind of
unnecessary.
> I decided to look at fire{BS|AS}Triggers() first, because we changed
> them in f49b85d783f to look at ModifyTableState.rootResultRelInfo to
> get the table whose triggers to fire. They seem broken for this case:
Yuck. But either proposal above would fix that, right?
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-24 19:07 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-25 08:16 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-24 19:07 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
I wrote:
> I suggest that it might be cleaner if we make rootRelation point
> to the original appendrel (that is, the RTE entry with inh = true).
> That would be exactly parallel to the partitioning situation, in
> that that RTE is not scanned in the plan. But it's for the same
> table as what's normally the first result table, so it should have
> the same permissions info.
After looking closer I see that that's exactly what's happening
in your patch, so it should all be good. We can make the code in
grouping_planner be simpler rather than more complicated, though.
I went ahead and pushed that to v14 and up, with adjustments of
relevant comments and a test case. There are still loose ends
here, though:
* The wrong-table-for-triggers bug occurs pre-v14, but the patch
doesn't come close to applying because both the planner and
executor look quite a bit different in this area. We could
devise a separate patch maybe, but given the lack of field
complaints I'm not sure this is worth doing. I'd be afraid
to put such a patch into v11 at this point anyway, given that
there will be no opportunity to fix any new bugs after November.
* I'm still seeing the extra RTEPermissionsInfo. It looks like that's
a consequence of this kluge in expand_single_inheritance_child:
/*
* No permission checking for the child RTE unless it's the parent
* relation in its child role, which only applies to traditional
* inheritance.
*/
if (childOID != parentOID)
childrte->perminfoindex = 0;
I suspect that now this should just unconditionally clear
childrte->perminfoindex, but it's minor cleanup not a bug fix
so I didn't pursue that in the initial patch.
* It seems like ModifyTable.nominalRelation and
ModifyTable.rootRelation are pretty darn redundant. Maybe we
should make an effort to get rid of one of them. Or maybe
it's not worth the trouble.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-25 08:16 ` Amit Langote <amitlangote09@gmail.com>
2023-10-25 13:51 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Amit Langote @ 2023-10-25 08:16 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Oct 25, 2023 at 4:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > I suggest that it might be cleaner if we make rootRelation point
> > to the original appendrel (that is, the RTE entry with inh = true).
> > That would be exactly parallel to the partitioning situation, in
> > that that RTE is not scanned in the plan. But it's for the same
> > table as what's normally the first result table, so it should have
> > the same permissions info.
>
> After looking closer I see that that's exactly what's happening
> in your patch, so it should all be good. We can make the code in
> grouping_planner be simpler rather than more complicated, though.
>
> I went ahead and pushed that to v14 and up, with adjustments of
> relevant comments and a test case.
Thanks.
> There are still loose ends
> here, though:
>
> * The wrong-table-for-triggers bug occurs pre-v14, but the patch
> doesn't come close to applying because both the planner and
> executor look quite a bit different in this area. We could
> devise a separate patch maybe, but given the lack of field
> complaints I'm not sure this is worth doing. I'd be afraid
> to put such a patch into v11 at this point anyway, given that
> there will be no opportunity to fix any new bugs after November.
Yeah, it might not be worthwhile to try to back-patch this to 12 and 13.
> * I'm still seeing the extra RTEPermissionsInfo. It looks like that's
> a consequence of this kluge in expand_single_inheritance_child:
>
> /*
> * No permission checking for the child RTE unless it's the parent
> * relation in its child role, which only applies to traditional
> * inheritance.
> */
> if (childOID != parentOID)
> childrte->perminfoindex = 0;
>
> I suspect that now this should just unconditionally clear
> childrte->perminfoindex, but it's minor cleanup not a bug fix
> so I didn't pursue that in the initial patch.
Would you like me to apply something like the attached?
> * It seems like ModifyTable.nominalRelation and
> ModifyTable.rootRelation are pretty darn redundant. Maybe we
> should make an effort to get rid of one of them. Or maybe
> it's not worth the trouble.
We had a discussion on unifying the two before:
https://www.postgresql.org/message-id/12148.1538938507%40sss.pgh.pa.us
I'm fine with leaving that as-is.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-Prevent-duplicate-RTEPermissionInfo-for-plain-inh.patch (application/octet-stream, 1.6K) download
From ac81db0f6033184ad7ad18dc83ccdeba8c052cb7 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 25 Oct 2023 16:51:22 +0900
Subject: [PATCH v1] Prevent duplicate RTEPermissionInfo for plain-inheritance
parents
Due to an oversight in a61b1f74823, expand_single_inheritance_child()
currently fails to reset perminfoindex in a plain-inheritance parent's
child RTE. That in turn causes add_rte_to_flat_rtable() to create a
duplicate RTEPermissionInfo for the parent's child RTE. Fix by making
expand_single_inheritance_child() always reset perminfoindex in the
child RTEs.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/839708.1698174464@sss.pgh.pa.us
Backpatch-through: 16
---
src/backend/optimizer/util/inherit.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index 94de855a22..00b65ca305 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -494,13 +494,8 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
childrte->inh = false;
childrte->securityQuals = NIL;
- /*
- * No permission checking for the child RTE unless it's the parent
- * relation in its child role, which only applies to traditional
- * inheritance.
- */
- if (childOID != parentOID)
- childrte->perminfoindex = 0;
+ /* No permission checking for child RTEs. */
+ childrte->perminfoindex = 0;
/* Link not-yet-fully-filled child RTE into data structures */
parse->rtable = lappend(parse->rtable, childrte);
--
2.35.3
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-25 08:16 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
@ 2023-10-25 13:51 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-26 02:27 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-25 13:51 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Amit Langote <amitlangote09@gmail.com> writes:
> On Wed, Oct 25, 2023 at 4:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I suspect that now this should just unconditionally clear
>> childrte->perminfoindex, but it's minor cleanup not a bug fix
>> so I didn't pursue that in the initial patch.
> Would you like me to apply something like the attached?
Diff looks fine, but I'm not sure that it's appropriate to characterize
the existing code as an oversight. It was a necessary hack while the
executor was behaving as it did (ie, using the first child as root).
>> * It seems like ModifyTable.nominalRelation and
>> ModifyTable.rootRelation are pretty darn redundant. Maybe we
>> should make an effort to get rid of one of them. Or maybe
>> it's not worth the trouble.
> We had a discussion on unifying the two before:
> https://www.postgresql.org/message-id/12148.1538938507%40sss.pgh.pa.us
Ah, so we did. The "serve different masters" argument did re-occur
to me while I was looking at this yesterday, but I'm not sure how
strong it is really. Anyway, I'm also content to leave it be.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-25 08:16 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-25 13:51 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-26 02:27 ` Amit Langote <amitlangote09@gmail.com>
2023-10-26 02:31 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 19+ messages in thread
From: Amit Langote @ 2023-10-26 02:27 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Oct 25, 2023 at 10:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
> > On Wed, Oct 25, 2023 at 4:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I suspect that now this should just unconditionally clear
> >> childrte->perminfoindex, but it's minor cleanup not a bug fix
> >> so I didn't pursue that in the initial patch.
>
> > Would you like me to apply something like the attached?
>
> Diff looks fine, but I'm not sure that it's appropriate to characterize
> the existing code as an oversight. It was a necessary hack while the
> executor was behaving as it did (ie, using the first child as root).
Ah, you're right. I've updated the commit message. Will push later today.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
Attachments:
v2-0001-Prevent-duplicate-RTEPermissionInfo-for-plain-inh.patch (application/octet-stream, 1.8K) download
From ba11e2762791360316e232a9d3194931779c55b4 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Thu, 26 Oct 2023 11:25:10 +0900
Subject: [PATCH v2] Prevent duplicate RTEPermissionInfo for plain-inheritance
parents
Currently, expand_single_inheritance_child() doesn't reset
perminfoindex in a plain-inheritance parent's child RTE, because
prior to 387f9ed0a0, the executor would use the child RTE to locate
the parent's RTEPermissionInfo. That in turn causes
add_rte_to_flat_rtable() to create an extra RTEPermissionInfo
belonging to the parent's child RTE with the same content as the one
belonging to the parent's original ("root") RTE.
In 387f9ed0a0, we changed things so that the executor can now use the
parent's "root" RTE for locating its RTEPermissionInfo, so the child
RTE's perminfoindex need not be set anymore, so make it so.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/839708.1698174464@sss.pgh.pa.us
Backpatch-through: 16
---
src/backend/optimizer/util/inherit.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index 94de855a22..00b65ca305 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -494,13 +494,8 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
childrte->inh = false;
childrte->securityQuals = NIL;
- /*
- * No permission checking for the child RTE unless it's the parent
- * relation in its child role, which only applies to traditional
- * inheritance.
- */
- if (childOID != parentOID)
- childrte->perminfoindex = 0;
+ /* No permission checking for child RTEs. */
+ childrte->perminfoindex = 0;
/* Link not-yet-fully-filled child RTE into data structures */
parse->rtable = lappend(parse->rtable, childrte);
--
2.35.3
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-25 08:16 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-25 13:51 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-26 02:27 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
@ 2023-10-26 02:31 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-26 03:01 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Tom Lane @ 2023-10-26 02:31 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Amit Langote <amitlangote09@gmail.com> writes:
> On Wed, Oct 25, 2023 at 10:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Diff looks fine, but I'm not sure that it's appropriate to characterize
>> the existing code as an oversight. It was a necessary hack while the
>> executor was behaving as it did (ie, using the first child as root).
> Ah, you're right. I've updated the commit message. Will push later today.
I'd suggest specifying *first* child RTE in the commit message.
LGTM otherwise.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 20:59 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-25 08:16 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-25 13:51 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
2023-10-26 02:27 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Amit Langote <amitlangote09@gmail.com>
2023-10-26 02:31 ` Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Tom Lane <tgl@sss.pgh.pa.us>
@ 2023-10-26 03:01 ` Amit Langote <amitlangote09@gmail.com>
0 siblings, 0 replies; 19+ messages in thread
From: Amit Langote @ 2023-10-26 03:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Hans Buschmann <buschmann@nidsa.net>; Peter Geoghegan <pg@bowt.ie>; David G. Johnston <david.g.johnston@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Oct 26, 2023 at 11:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
> > On Wed, Oct 25, 2023 at 10:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Diff looks fine, but I'm not sure that it's appropriate to characterize
> >> the existing code as an oversight. It was a necessary hack while the
> >> executor was behaving as it did (ie, using the first child as root).
>
> > Ah, you're right. I've updated the commit message. Will push later today.
>
> I'd suggest specifying *first* child RTE in the commit message.
> LGTM otherwise.
Thanks for checking. Pushed after changing the commit message like that.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2023-10-26 03:01 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-10-23 17:05 AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx Hans Buschmann <buschmann@nidsa.net>
2023-10-23 18:30 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 18:49 ` Peter Geoghegan <pg@bowt.ie>
2023-10-23 18:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-23 19:01 ` Peter Geoghegan <pg@bowt.ie>
2023-10-23 22:15 ` Peter Geoghegan <pg@bowt.ie>
2023-10-24 00:28 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 00:45 ` Peter Geoghegan <pg@bowt.ie>
2023-10-24 01:06 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 01:23 ` Peter Geoghegan <pg@bowt.ie>
2023-10-23 20:59 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 09:48 ` Amit Langote <amitlangote09@gmail.com>
2023-10-24 14:48 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-24 19:07 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-25 08:16 ` Amit Langote <amitlangote09@gmail.com>
2023-10-25 13:51 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-26 02:27 ` Amit Langote <amitlangote09@gmail.com>
2023-10-26 02:31 ` Tom Lane <tgl@sss.pgh.pa.us>
2023-10-26 03:01 ` Amit Langote <amitlangote09@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox