On Mon, Sep 29, 2025 at 9:59 PM Richard Guo <guofenglinux@gmail.com> wrote:
> I don't think any of the bugs you described upthread exist in the PoC
> patch. The patch ensures that all names stored in glob->subplanNames
> are canonicalized to the format "$basename_$suffixnum". If no numeric
> suffix is required, the name is stored with a "_0" suffix. This
> guarantees a clear distinction between the base name and the numeric
> suffix for all names stored in the list. (Please note that this
> canonicalization applies only to how names are stored internally, not
> to user-visible names. So no user-visible behavior change here.)
I studied your patch in more detail today and I find that you are
correct. It doesn't do what I thought it did. However, it also doesn't
guarantee uniqueness. Here is an example:
robert.haas=# explain select (select random() limit 1), (select
random() limit 2) from (select * from pg_class limit 1) expr_1;
WARNING: choose_plan_name for name "expr" returns "expr_1"
WARNING: choose_plan_name for name "expr" returns "expr_2"
WARNING: choose_plan_name for name "expr_1" returns "expr_1"
QUERY PLAN
-------------------------------------------------------------------------
Subquery Scan on expr_1 (cost=0.03..0.08 rows=1 width=16)
InitPlan expr_1
-> Limit (cost=0.00..0.01 rows=1 width=8)
-> Result (cost=0.00..0.01 rows=1 width=8)
InitPlan expr_2
-> Limit (cost=0.00..0.01 rows=1 width=8)
-> Result (cost=0.00..0.01 rows=1 width=8)
-> Limit (cost=0.00..0.04 rows=1 width=240)
-> Seq Scan on pg_class (cost=0.00..18.16 rows=416 width=240)
(9 rows)
When I originally looked at the patch, I believed that the use of
atoi() without sanity checking was going to cause the sorts of
problems I was mentioning. That was wrong, since you never let a
user-specified name leak directly into the list, but only names to
which your code has added a numeric suffix. But that also means that
you don't get conflicts between names that, in reality, should
conflict.
--
Robert Haas
EDB: http://www.enterprisedb.com
