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

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.


I doubt [1] your change will solve all the problems.
[1] https://postgr.es/m/CAHewXNnKLrZYG4iqaYw=uB3XWRrYRZHo7VtcMsbUEbdbajQg2Q@mail.gmail.com