On 2017/01/21 9:01, Tom Lane wrote:
> Robert Haas <robertmh...@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 =
> 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.
Sorry for jumping in late. Attached patch replaces the call to
partitioning-specific comparison function by the call to datumIsEqual().
I wonder if it is safe to assume that datumIsEqual() would return true for
a datum and copy of it made using datumCopy(). The latter is used to copy
a single datum from a bound's Const node (what is stored in the catalog
for every bound).
> 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.
I assume "making datumIsEqual() smarter" here means to make it account for
toasting of varlena datums, which is not a good idea because some of its
callers may be working in the context of an aborted transaction. I tried
to update the header comment along these lines, though please feel to
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index ad95b1bc55..8c9a373f1f 100644
@@ -639,12 +639,14 @@ partition_bounds_equal(PartitionKey key,
- /* Compare the actual values */
- cmpval = DatumGetInt32(FunctionCall2Coll(&key->partsupfunc[j],
- if (cmpval != 0)
+ * Compare the actual values; note that we do not invoke the
+ * partitioning specific comparison operator here, because we
+ * could be in an aborted transaction.
+ if (!datumIsEqual(b1->datums[i][j], b2->datums[i][j],
diff --git a/src/backend/utils/adt/datum.c b/src/backend/utils/adt/datum.c
index 535e4277cc..071a7d4db1 100644
@@ -209,6 +209,10 @@ datumTransfer(Datum value, bool typByVal, int typLen)
* of say the representation of zero in one's complement arithmetic).
* Also, it will probably not give the answer you want if either
* datum has been "toasted".
+ * Do not try to make this any smarter than it currently is with respect
+ * to "toasted" datums, because some of the callers could be working in the
+ * context of an aborted transaction.
Sent via pgsql-hackers mailing list (firstname.lastname@example.org)
To make changes to your subscription: