Re: BUG #18568: BUG: Result wrong when do group by on partition table!

jian he <jian.universality@gmail.com> 于2024年10月23日周三 22:18写道：

> On Wed, Oct 23, 2024 at 6:43 PM Tender Wang <tndrwang@gmail.com> wrote:
> >
> > I tried the patch I provided in [1], and the regression test cases all
> passed.
> >
>
> ////////////////////////
> ComputePartitionAttrs code snippet
> ELSE
> {
>             /* Expression */
>             Node       *expr = pelem->expr;
>             char        partattname[16];
>             Assert(expr != NULL);
>             atttype = exprType(expr);
>             attcollation = exprCollation(expr);
> }
>         /*
>          * Apply collation override if any
>          */
>         if (pelem->collation)
>             attcollation = get_collation_oid(pelem->collation, false);
>         partcollation[attn] = attcollation;
> ////////////////////////
>
> create table coll_pruning_multi (a text) partition by range (substr(a,
> 1) collate "POSIX", substr(a, 1) collate "C");
> PartitionElem->expr only cover "substr(a,1)".
> PartitionElem->collation is for explicitly COLLATION clauses.
> you can also see
>
> https://github.com/postgres/postgres/blob/master/src/backend/parser/gram.y#L4556
>
> From the above "collation override" comments, we can say
> exprCollation(PartitionElem->expr)
> does not always equal PartitionElem->collation
> PartitionElem->collation is the true collation OID.
>
> so you change in but didn't cover the ELSE branch.
>
>         else
>         {
>             if (lc == NULL)
>                 elog(ERROR, "wrong number of partition key expressions");
>             /* Re-stamp the expression with given varno. */
>             partexpr = (Expr *) copyObject(lfirst(lc));
>             ChangeVarNodes((Node *) partexpr, 1, varno, 0);
>             lc = lnext(partkey->partexprs, lc);
>         }
>
> as you mentioned partkey->partcollation is correct collation for
> PartitionKey.
> but the ELSE branch, we cannot do
>         else
>         {
>             if (lc == NULL)
>                 elog(ERROR, "wrong number of partition key expressions");
>             /* Re-stamp the expression with given varno. */
>             partexpr = (Expr *) copyObject(lfirst(lc));
>             ChangeVarNodes((Node *) partexpr, 1, varno, 0);
>            exprSetCollation(Node *partexpr, Oid collation)
>             lc = lnext(partkey->partexprs, lc);
>         }
>
> because in struct inPartitionElem, collation and expr is seperated.
> that means after set_baserel_partition_key_exprs
> we still cannot be sure that RelOptInfo->partexprs have the correct
> PartitionKey collation information.
>

Yeah, you're right. I confirm this again. In
set_baserel_partition_key_exprs(),
we copy partkey->partexprs not including partcollation, if it is not simple
column reference.

So I think how we can fix this thread issue and the [1] I reported by me
using a uniform solution.
By the way, I re-started a new thread [2] to track the issue I found in
[1]. I will reply to an email reflecting what you said here and cc you.

[1]
https://www.postgresql.org/message-id/CAHewXNnyWUEmdHrRK3yg4k2TzSbb5WnkKLWxyO%2BOVZPhPFX7ew%40mail.gmail.com

[2]
https://www.postgresql.org/message-id/CAHewXNno_HKiQ6PqyLYfuqDtwp7KKHZiH1J7Pqyz0nr%2BPS2Dwg%40mail.gmail.com

-- 
Thanks,
Tender Wang