Re: AW: AW: BUG #18147: ERROR: invalid perminfoindex 0 in RTE with relid xxxxx

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