On Sun, 5 Apr 2020 at 18:45, Julien Rouhaud <rjuju...@gmail.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