On Fri, Jul 29, 2022 at 6:26 PM Ashutosh Sharma <ashu.coe...@gmail.com> wrote:
> On Thu, Jul 28, 2022 at 5:02 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > +/* ---------- > + * RelFileNumber zero is InvalidRelFileNumber. > + * > + * For the system tables (OID < FirstNormalObjectId) the initial storage > > Above comment says that RelFileNumber zero is invalid which is technically > correct because we don't have any relation file in disk with zero number. > But the point is that if someone reads below definition of > CHECK_RELFILENUMBER_RANGE he/she might get confused because as per this > definition relfilenumber zero is valid. > Please ignore the above comment shared in my previous email. It is a little over-thinking on my part that generated this comment in my mind. Sorry for that. Here are the other comments I have: +/* First we have to remove them from the extension */ +ALTER EXTENSION pg_buffercache DROP VIEW pg_buffercache; +ALTER EXTENSION pg_buffercache DROP FUNCTION pg_buffercache_pages(); + +/* Then we can drop them */ +DROP VIEW pg_buffercache; +DROP FUNCTION pg_buffercache_pages(); + +/* Now redefine */ +CREATE OR REPLACE FUNCTION pg_buffercache_pages() +RETURNS SETOF RECORD +AS 'MODULE_PATHNAME', 'pg_buffercache_pages_v1_4' +LANGUAGE C PARALLEL SAFE; + +CREATE OR REPLACE VIEW pg_buffercache AS + SELECT P.* FROM pg_buffercache_pages() AS P + (bufferid integer, relfilenode int8, reltablespace oid, reldatabase oid, + relforknumber int2, relblocknumber int8, isdirty bool, usagecount int2, + pinning_backends int4); As we are dropping the function and view I think it would be good if we *don't* use the "OR REPLACE" keyword when re-defining them. == + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("relfilenode" INT64_FORMAT " is too large to be represented as an OID", + fctx->record[i].relfilenumber), + errhint("Upgrade the extension using ALTER EXTENSION pg_buffercache UPDATE"))); I think it would be good to recommend users to upgrade to the latest version instead of just saying upgrade the pg_buffercache using ALTER EXTENSION .... == --- a/contrib/pg_walinspect/sql/pg_walinspect.sql +++ b/contrib/pg_walinspect/sql/pg_walinspect.sql @@ -39,10 +39,10 @@ SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1'); -- Test for filtering out WAL records of a particular table -- =================================================================== -SELECT oid AS sample_tbl_oid FROM pg_class WHERE relname = 'sample_tbl' \gset +SELECT relfilenode AS sample_tbl_relfilenode FROM pg_class WHERE relname = 'sample_tbl' \gset Is this change required? The original query is just trying to fetch table oid not relfilenode and AFAIK we haven't changed anything in table oid. == +#define CHECK_RELFILENUMBER_RANGE(relfilenumber) \ +do { \ + if ((relfilenumber) < 0 || (relfilenumber) > MAX_RELFILENUMBER) \ + ereport(ERROR, \ + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), \ + errmsg("relfilenumber " INT64_FORMAT " is out of range", \ + (relfilenumber)))); \ +} while (0) + I think we can shift this macro to some header file and reuse it at several places. == + * Generate a new relfilenumber. We cannot reuse the old relfilenumber + * because of the possibility that that relation will be moved back to the that that relation -> that relation -- With Regards, Ashutosh Sharma.