On 2013-12-10 19:11:03 -0500, Robert Haas wrote: > Committed #1 (again). Regarding this: > > + /* XXX: we could also do this unconditionally, the space is used > anyway > + if (copy_oid) > + HeapTupleSetOid(key_tuple, HeapTupleGetOid(tp)); > > I would like to put in a big +1 for doing that unconditionally. I > didn't make that change before committing, but I think it'd be a very > good idea.
Patch attached. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
>From 47c93d8e7afbcaef268de66246571cdc2f134c97 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Wed, 11 Dec 2013 17:20:27 +0100 Subject: [PATCH] Dep't of second thoughts: Always include oids in WAL logged replica identities. Since replica identities are logged using the normal format for heap tuples, the space for oids in WITH OIDS tables was already used, so there's little point in only including the oid if it is included in the configured IDENTITY. Per comment from Robert Haas. --- src/backend/access/heap/heapam.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 249fffe..e647453 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -6638,7 +6638,6 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool * TupleDesc idx_desc; char replident = relation->rd_rel->relreplident; HeapTuple key_tuple = NULL; - bool copy_oid = false; bool nulls[MaxHeapAttributeNumber]; Datum values[MaxHeapAttributeNumber]; int natt; @@ -6698,7 +6697,8 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool * int attno = idx_rel->rd_index->indkey.values[natt]; if (attno == ObjectIdAttributeNumber) - copy_oid = true; + /* copied below */ + ; else if (attno < 0) elog(ERROR, "system column in index"); else @@ -6709,8 +6709,12 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool * *copy = true; RelationClose(idx_rel); - /* XXX: we could also do this unconditionally, the space is used anyway */ - if (copy_oid) + /* + * Always copy oids if the table has them, even if not included in the + * index. The space in the logged tuple is used anyway, so there's little + * point in not including the information. + */ + if (relation->rd_rel->relhasoids) HeapTupleSetOid(key_tuple, HeapTupleGetOid(tp)); /* -- 1.8.5.rc2.dirty
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
