Re: Valgrind-detected bug in partitioning code

Robert Haas <robertmhaas(at)gmail(dot)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