On 3/24/21 2:36 PM, Dean Rasheed wrote: > On Wed, 24 Mar 2021 at 10:22, Tomas Vondra > <tomas.von...@enterprisedb.com> wrote: >> >> Thanks, it seems to be some thinko in handling in PlaceHolderVars, which >> seem to break the code's assumptions about varnos. This fixes it for me, >> but I need to look at it more closely. >> > > I think that makes sense. >
AFAIK the primary issue here is that the two places disagree. While estimate_num_groups does this varnos = pull_varnos(root, (Node *) varshere); if (bms_membership(varnos) == BMS_SINGLETON) { ... } the add_unique_group_expr does this varnos = pull_varnos(root, (Node *) groupexpr); That is, one looks at the group expression, while the other look at vars extracted from it by pull_var_clause(). Apparently for PlaceHolderVar this can differ, causing the crash. So we need to change one of those places - my fix tweaked the second place to also look at the vars, but maybe we should change the other place? Or maybe it's not the right fix for PlaceHolderVars ... > Reviewing the docs, I noticed a couple of omissions, and had a few > other suggestions (attached). > Thanks! I'll include that in the next version of the patch. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company