Re: BUG #18568: BUG: Result wrong when do group by on partition table! - Mailing list pgsql-bugs

From Tender Wang
Subject Re: BUG #18568: BUG: Result wrong when do group by on partition table!
Date
Msg-id CAHewXNn6LuM5m8tiBQE8SPZiZv4PmsxmAoOYSKWy9C0zH32JUQ@mail.gmail.com
Whole thread Raw
In response to Re: BUG #18568: BUG: Result wrong when do group by on partition table!  (jian he <jian.universality@gmail.com>)
Responses Re: BUG #18568: BUG: Result wrong when do group by on partition table!
List pgsql-bugs


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.


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

--
Thanks,
Tender Wang

pgsql-bugs by date:

Previous
From: PG Bug reporting form
Date:
Subject: BUG #18669: pg_get_shmem_allocations show wrong memory for 11 structures
Next
From: PG Bug reporting form
Date:
Subject: BUG #18670: Can't install success, the cluster always failed