Re: [HACKERS] Re: [COMMITTERS] pgsql: Prevent "snapshot too old" from trying to return pruned TOAST tu
On Fri, Aug 5, 2016 at 11:05 AM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Aug 4, 2016 at 10:58 PM, Robert Haas wrote: >>> What is the reason? We refuse to separate frontend and backend >>> headers in any sort of principled way? > >> That was poorly phrased. I'll try again: I can't see any reason for >> using a macro here except that it allows frontend code to compile this >> without breaking. > > Well, the alternative would be to put "#ifndef FRONTEND" around the > static-inline function. That's not very pretty either, and it's > inconsistent with the existing precedent (ie, InitDirtySnapshot). > Also, it presumes that a non-backend includer actually has defined > FRONTEND; that seems to be the case for pg_xlogdump but I do not > think we do that everywhere. That may not be pretty, but it'd be a lot more transparent. If I see #ifndef FRONTEND, I think, oh, that's protecting some stuff that shouldn't be included in front-end compiles. If I see a macro, I not necessarily think of the fact that this may be a way of preventing that code from being compiled in front-end compiles. >> Here's a patch. Is this what you want? > > OK by me. OK, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Prevent "snapshot too old" from trying to return pruned TOAST tu
Robert Haas writes: > On Thu, Aug 4, 2016 at 10:58 PM, Robert Haas wrote: >> What is the reason? We refuse to separate frontend and backend >> headers in any sort of principled way? > That was poorly phrased. I'll try again: I can't see any reason for > using a macro here except that it allows frontend code to compile this > without breaking. Well, the alternative would be to put "#ifndef FRONTEND" around the static-inline function. That's not very pretty either, and it's inconsistent with the existing precedent (ie, InitDirtySnapshot). Also, it presumes that a non-backend includer actually has defined FRONTEND; that seems to be the case for pg_xlogdump but I do not think we do that everywhere. > Here's a patch. Is this what you want? OK by me. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Prevent "snapshot too old" from trying to return pruned TOAST tu
On Thu, Aug 4, 2016 at 10:58 PM, Robert Haas wrote: > On Thu, Aug 4, 2016 at 7:23 PM, Tom Lane wrote: >> Robert Haas writes: >>> Prevent "snapshot too old" from trying to return pruned TOAST tuples. >> >> Looks like this patch broke the build on castoroides. Recommend >> changing InitToastSnapshot into a macro. (There's a reason why >> InitDirtySnapshot is a macro.) > > What is the reason? We refuse to separate frontend and backend > headers in any sort of principled way? That was poorly phrased. I'll try again: I can't see any reason for using a macro here except that it allows frontend code to compile this without breaking. But that doesn't seem like a very good way of solving that problem. There's surely no way for a casual reader of the code to realize that macros can be used here and inline functions cannot, especially because this works apparently works fine on most machines, including mine. Here's a patch. Is this what you want? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index 55b6b41..31b0132 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -2316,5 +2316,5 @@ init_toast_snapshot(Snapshot toast_snapshot) if (snapshot == NULL) elog(ERROR, "no known snapshots"); - InitToastSnapshot(toast_snapshot, snapshot->lsn, snapshot->whenTaken); + InitToastSnapshot(*toast_snapshot, snapshot->lsn, snapshot->whenTaken); } diff --git a/src/include/utils/tqual.h b/src/include/utils/tqual.h index 8041e7b..fc7328c 100644 --- a/src/include/utils/tqual.h +++ b/src/include/utils/tqual.h @@ -104,12 +104,9 @@ extern bool ResolveCminCmaxDuringDecoding(struct HTAB *tuplecid_data, * Similarly, some initialization is required for SnapshotToast. We need * to set lsn and whenTaken correctly to support snapshot_too_old. */ -static inline void -InitToastSnapshot(Snapshot snapshot, XLogRecPtr lsn, int64 whenTaken) -{ - snapshot->satisfies = HeapTupleSatisfiesToast; - snapshot->lsn = lsn; - snapshot->whenTaken = whenTaken; -} +#define InitToastSnapshot(snapshotdata, l, w) \ + ((snapshotdata).satisfies = HeapTupleSatisfiesDirty, \ + (snapshotdata).lsn = (l), \ + (snapshotdata).whenTaken = (w)) #endif /* TQUAL_H */ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers