On Fri, Jul 29, 2022 at 8:02 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
>
> + 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_buffercacheusing ALTER EXTENSION ....
This error would be hit if the relfilenumber is out of OID range that
means the user is using a new cluster but old pg_buffercache
extension. So this errhint is about suggesting to upgrade the
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
changedanything in table oid.
If you notice the complete test, then you will realize that
sample_tbl_oid are used for verifying that in
pg_get_wal_records_info(), so earlier it was okay if we were using oid
instead of relfilenode because this test case is just creating table
doing some DML and verifying oid in WAL so that will be same as
relfilenode, but that is no longer true. So we will have to check the
relfilenode that was the actual intention of the test.
>
> + * 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
>
I think this is a grammatically correct sentence .
I have fixed other comments, and also fixed comments from Alvaro to
use %lld instead of INT64_FORMAT inside the ereport and wherever he
suggested.
I haven't yet changed MAX_RELFILENUMBER to represent the hex
characters because then we will have to change the filename as well.
So I think there is no conclusion on this yet whether we want to keep
it as it is or in hex. And there is another suggestion to change one
of the existing elog to an ereport, so for that I will share a
separate patch.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
