Hi Jim, I tested v3 on a primary/standby pair built on top of eaa5961efc6. All five patches apply cleanly and build without warnings. On the standby, every new check I tried reports the new error for all the functions you reported. get_raw_page(..., 'init', 0) on an unlogged index still works, as intended.
A few comments:
1) In 0001, the detoast guard checks the relation passed by the caller,
but detoasting follows va_toastrelid from the pointer itself, so the two
can disagree. In addition to Kirill's u_toast/u_snapshot, I
created on the primary:
CREATE TABLE logged_text (b text);
CREATE TABLE l_toast (b text);
ALTER TABLE l_toast ALTER COLUMN b SET STORAGE EXTERNAL;
INSERT INTO l_toast VALUES (repeat('abc', 3000));
CREATE TABLE l_snapshot AS SELECT get_raw_page('l_toast', 0) AS raw;
Then on the standby:
-- page from an unlogged table, permanent table passed in
SELECT octet_length(t_attrs[1]) FROM u_snapshot,
LATERAL heap_page_item_attrs(raw, 'logged_text'::regclass, true);
ERROR: could not open file "base/5/16468": No such file or directory
-- page from a permanent table, unlogged table passed in
SELECT octet_length(t_attrs[1]) FROM l_snapshot,
LATERAL heap_page_item_attrs(raw, 'u_toast'::regclass, true);
ERROR: cannot access temporary or unlogged relations during recovery
The second value is readable on the standby. Checking the TOAST relation
from the pointer fixes both, and still catches Kirill's case:
if (VARATT_IS_EXTERNAL_ONDISK(tupdata + off) &&
RecoveryInProgress())
{
toast_external_data toast_ext_data;
toast_external_info_get((varlena *) (tupdata + off),
&toast_ext_data);
/* If the relation is gone, let detoasting complain. */
if (SearchSysCacheExists1(RELOID,
ObjectIdGetDatum(toast_ext_data.toastrelid)) &&
get_rel_persistence(toast_ext_data.toastrelid) !=
RELPERSISTENCE_PERMANENT)
ereport(ERROR, ...);
}
toast_external_info_get() handles both the OID and OID8 pointer formats.
The existence check matters for a saved page whose TOAST
table has since been dropped, without it get_rel_persistence() fails with
"cache lookup failed for relation", while with it the error stays
"could not open relation with OID", as in v3.
2) pg_freespacemap has the same problem as pg_visibility_map*. On the
standby, pg_freespace('ut', 0) returns 0 while the primary returns 3968.
For another session's temp table, it returns 0 if the table has
no FSM fork yet, but fails with "cannot access temporary tables of other
sessions" once it has one. Attached is 0006, which adds both
checks to pg_freespace(), as 0004 and 0005 do for pg_visibility; the
module's regression test passes with it.
3) Regarding Michael's point about duplication, pg_visibility now has a
single check, but pgstattuple still carries six copies and pageinspect
three (plus the variants in get_raw_page and the detoast path), and core
has three more of the same shape (plancat.c, copyto.c,
tid.c). Would a small core helper make sense here, something like
CheckRelationReadableDuringRecovery(Relation), so all of these share one
place and one message?
A minor comment, pg_prewarm on an unlogged table on a standby reports
'fork "main" does not exist for this relation', which is not wrong but
could use the same check and message as the other modules.
Regards,
Surya Poondla
0006-pg_freespacemap-Reject-temp-unlogged-relations-it-ca.patch
Description: Binary data
