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

jian he <jian(dot)universality(at)gmail(dot)com> 于2024年10月23日周三 22:18写道：

> On Wed, Oct 23, 2024 at 6:43 PM Tender Wang <tndrwang(at)gmail(dot)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