Re: [HACKERS] Valgrind-detected bug in partitioning code - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] Valgrind-detected bug in partitioning code
Date
Msg-id 31998.1484956908@sss.pgh.pa.us
Whole thread Raw
In response to Re: [HACKERS] Valgrind-detected bug in partitioning code  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: [HACKERS] Valgrind-detected bug in partitioning code  (Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [HACKERS] increasing the default WAL segment size
Next
From: Thomas Munro
Date:
Subject: [HACKERS] remote_apply for logical replication?