Re: Online checksums verification in the backend

On Sun, 5 Apr 2020 at 18:45, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> On Sun, Apr 05, 2020 at 06:08:06PM +0900, Masahiko Sawada wrote:
> >
> > Why do we need two rows in the doc? For instance, replication slot
> > functions have some optional arguments but there is only one row in
> > the doc. So I think we don't need to change the doc from the previous
> > version patch.
> >
>
> I thought that if we document the function as pg_check_relation(regclass [,
> fork]) users could think that the 2nd argument is optional, so that
> pg_check_relation('something', NULL) could be a valid alias for the 1-argument
> form, which it isn't. After checking, I see that e.g. current_setting has the
> same semantics and is documented the way you suggest, so fixed back to previous
> version.
>
> > And I think these are not necessary as we already defined in
> > include/catalog/pg_proc.dat:
> >
> > +CREATE OR REPLACE FUNCTION pg_check_relation(
> > + IN relation regclass,
> > + OUT relid oid, OUT forknum integer, OUT failed_blocknum bigint,
> > + OUT expected_checksum integer, OUT found_checksum integer)
> > + RETURNS SETOF record STRICT VOLATILE LANGUAGE internal AS 'pg_check_relation'
> > + PARALLEL RESTRICTED;
> > +
> > +CREATE OR REPLACE FUNCTION pg_check_relation(
> > + IN relation regclass, IN fork text,
> > + OUT relid oid, OUT forknum integer, OUT failed_blocknum bigint,
> > + OUT expected_checksum integer, OUT found_checksum integer)
> > + RETURNS SETOF record STRICT VOLATILE LANGUAGE internal
> > + AS 'pg_check_relation_fork'
> > + PARALLEL RESTRICTED;
> >
>
> Oh right this isn't required since there's no default value anymore, fixed.
>
> v9 attached.

Thank you for updating the patch! The patch looks good to me.

I've marked this patch as Ready for Committer. I hope this patch will
get committed to PG13.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services