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 <alvhe...@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.gc26...@telsasoft.com>
Re: shadow variables - pg15 edition
--
Justin
