FOR EACH ROW triggers, on partitioend tables, with indexes? - Mailing list pgsql-hackers

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