On Thu, Apr 16, 2015 at 2:18 AM, Andres Freund <and...@anarazel.de> wrote: > On 2015-04-15 18:53:15 +0300, Heikki Linnakangas wrote: >> Hmm, ok, I've read the "INSERT ... ON CONFLICT UPDATE and logical decoding" >> thread now, and I have to say that IMHO it's a lot more sane to handle this >> in ReorderBufferCommit() like Peter first did, than to make the main >> insertion path more complex like this. > > I don't like Peter's way much. For one it's just broken. For another > it's quite annoying to trigger disk reads to figure out what actual type > of record something is. > > If we go that way that's the way I think it should be done: Whenever we > encounter a speculative record we 'unlink' it from the changes that will > be reused for spooling from disk and do nothing further. Then we just > continue reading through the records.
You mean we continue iterating through *changes* from ReorderBufferCommit()? > If the next thing we encounter is > a super-deletion we throw away that record. If it's another type of > change (or even bettter a 'speculative insertion succeeded' record) > insert it. By "insert it", I gather you mean report to the the logical decoding plugin as an insert change. > That'll still require some uglyness, but it should not be too > bad. So, to be clear, what you have in mind is sort of a hybrid between my first and second approaches (mostly my first approach). We'd have coordination between records originally decoded into changes, maybe "intercepting" them during xact reassembly, like my first approach. We'd mostly do the same thing as the first approach, actually. The major difference would be that there'd be explicit "speculative affirmation" WAL records. But we wouldn't rely on those affirmation records within ReorderBufferCommit() - we'd rely on the *absence* of a super deletion WAL record (to report an insert change to the decoding plugin). To emphasize, like my first approach, it would be based on an *absence* of a super deletion WAL record. However, like my second approach, there would be a "speculative affirmation" WAL record. The new speculative affirmation WAL record would however be quite different to what my second approach to logical decoding (the in-place update record thing that was in V3.3) actually did. In particular, it would be far more simple, and the tuple would be built from the original insertion record within logical decoding. Right now, I'm tired, so bear with me. What I think I'm not quite getting here is how the new type of "affirmation" record affects visibility (or anything else, actually). Clearly dirty snapshots need to see the record to set a speculative insertion token for their caller to wait on (and HeapTupleSatisfiesVacuum() cannot see the speculatively inserted tuple as reclaimable, of course). They need this *before* the speculative insertion is affirmed, of course. Maybe you mean: If the speculative insertion xact is in progress, then the tuple is visible to several types of snapshots (dirty, vacuum, self, any). If it is not, then tuples are not visible because they are only speculative (and not *confirmed*). If they were confirmed, and the xact was committed, then those tuples are logically and physically indistinguishable from tuples that were inserted in the ordinary manner. Is that it? Basically, the affirmation records have nothing much to do with logical decoding in particular. But you still need to super delete, so that several types of snapshots ((dirty, vacuum, self, any) *stop* seeing the tuple as visible independent of the inserting xact being in progress. > I earlier thought that'd not be ok because there could be a new catalog > snapshot inbetween, but I was mistaken: The locking on the source > transaction prevents problems. I thought this was the case. -- 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
