Re: [HACKERS] Valgrind-detected bug in partitioning code

Robert Haas <robertmhaas@gmail.com> writes:
> The difference is that those other equalBLAH functions call a
> carefully limited amount of code whereas, in looking over the
> backtrace you sent, I realized that equalPartitionDescs is calling
> partition_bounds_equal which does this:
>                        cmpval =
> DatumGetInt32(FunctionCall2Coll(&key->partsupfunc[j],
>                                   key->partcollation[j],
>                                   b1->datums[i][j],
>                                   b2->datums[i][j]))

Ah, gotcha.

> That's of course opening up a much bigger can of worms.  But apart
> from the fact that it's unsafe, I think it's also wrong, as I said
> upthread.  I think calling datumIsEqual() there should be better all
> around.  Do you think that's unsafe here?

That sounds like a plausible solution.  It is safe in the sense of
being a bounded amount of code.  It would return "false" in various
interesting cases like toast pointer versus detoasted equivalent,
but I think that would be fine in this application.

It would probably be a good idea to add something to datumIsEqual's
comment to the effect that trying to make it smarter would be a bad idea,
because some callers rely on it being stupid.
        regards, tom lane