On Tue, Mar 15, 2016 at 8:31 AM, Stephen Frost <sfr...@snowman.net> wrote: >> > We wouldn't want to end up returning different error messages for the >> > same command under the same conditions just based, which is what we'd >> > potentially end up doing if we used XLTW_InsertIndexUnique here. >> >> Perhaps we need a new value in that enum, so that the context message is >> specific to the situation at hand? > > Maybe, but that would require a new message and new translation and just > generally doesn't seem like something we'd want to back-patch for a > bugfix.
Thinking about this again, I think we should use XLTW_InsertIndexUnique after all. The resemblance of the check_exclusion_or_unique_constraint() code to the nbtinsert.c code seems only superficial on second thought. So, I propose fixing the fix by changing XLTW_InsertIndex to XLTW_InsertIndexUnique. Basically, unlike with the similar nbtinsert.c code, we're checking someone else's tuple in the speculative insertion check_exclusion_or_unique_constraint() case that was changed (or it's an exclusion constraint, where even the check for our own tuple happens only after insertion; no change there in any case). Whereas, in the nbtinsert.c case that I incorrectly duplicated, we're specifically indicating that we're waiting on *our own* already physically inserted heap tuple, and say as much in the XLTW_InsertIndex message that makes it into the log. So, the fd658dbb300456b393536802d1145a9cea7b25d6 fix is wrong in that we now indicate that the wait was on our own already-inserted tuple, and not *someone else's* already-inserted tuple, as it should (we haven't inserting anything in the first phase of speculative insertion, this precheck). This code is not a do-over of the check in nbtinsert.c -- rather, the code in nbtinsert.c is a second phase do-over of this code (where we've physically inserted a heap tuple + index tuple -- "speculative" though that is). It seems fine to characterize a wait here as "checking the uniqueness of [somebody else's] tuple", even though technically we're checking the would-be uniqueness were we to (speculatively, or actually) insert. However, it does not seem fine to claim ctid_wait as a tuple we ourselves inserted. Sorry about that. My confusion came from the fact that historically, when check_exclusion_or_unique_constraint() was called check_exclusion_constraint(), it (almost) was our own tuple that was waited on. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
