FOR EACH ROW triggers, on partitioend tables, with indexes?

On Wed, Aug 17, 2022 at 09:54:34AM -0500, Justin Pryzby wrote:
> But an unfortunate consequence of not fixing the historic issues is that it
> precludes the possibility that anyone could be expected to notice if they
> introduce more instances of the same problem (as in the first half of these
> patches).  Then the hole which has already been dug becomes deeper, further
> increasing the burden of fixing the historic issues before being able to use
> -Wshadow.
> 
> The first half of the patches fix shadow variables newly-introduced in v15
> (including one of my own patches), the rest are fixing the lowest hanging fruit
> of the "short list" from COPT=-Wshadow=compatible-local
> 
> I can't see that any of these are bugs, but it seems like a good goal to move
> towards allowing use of the -Wshadow* options to help avoid future errors, as
> well as cleanliness and readability (rather than allowing it to get harder to
> use -Wshadow).

+Alvaro

You wrote:

|commit 86f575948c773b0ec5b0f27066e37dd93a7f0a96
|Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
|Date:   Fri Mar 23 10:48:22 2018 -0300
|
|    Allow FOR EACH ROW triggers on partitioned tables

Which added:

     1	+       partition_recurse = !isInternal && stmt->row &&
     2	+           rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE;
     3	...
     4	+       if (partition_recurse)
     5	...
     6	+           List       *idxs = NIL;
     7	+           List       *childTbls = NIL;
     8	...
     9	+           if (OidIsValid(indexOid))
    10	+           {
    11	+               ListCell   *l;
    12	+               List       *idxs = NIL;
    13	+
    14	+               idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock);
    15	+               foreach(l, idxs)
    16	+                   childTbls = lappend_oid(childTbls,
    17	+                                           IndexGetRelation(lfirst_oid(l),
    18	+                                                        false));
    19	+           }
    20	...
    21	+           for (i = 0; i < partdesc->nparts; i++)
    22	...
    23	+               if (OidIsValid(indexOid))
    24	...
    25	+                   forboth(l, idxs, l2, childTbls)

The inner idxs is set at line 12, but the outer idxs being looped over at line
25 is still NIL, because the variable is shadowed.

That would be a memory leak or some other bug, except that it also seems to be
dead code ?

https://coverage.postgresql.org/src/backend/commands/trigger.c.gcov.html#1166

Is it somwhow possible to call CreateTrigger() to create a FOR EACH ROW
trigger, with an index, and not internally ?

The comments say that a user's CREATE TRIGGER will not have a constraint, so
won't have an index.

  * constraintOid is zero when
  * executing a user-entered CREATE TRIGGER command.
  *
+ * indexOid, if nonzero, is the OID of an index associated with the constraint.
+ * We do nothing with this except store it into pg_trigger.tgconstrindid.

See also: <20220817145434.GC26426@telsasoft.com>
Re: shadow variables - pg15 edition

-- 
Justin