public inbox for [email protected]  

help / color / mirror / Atom feed

From: Stephen Frost <[email protected]>
To: Tom Lane <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Fabien COELHO <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: David Steele <[email protected]>
Cc: [email protected]
Cc: Bossart, Nathan <[email protected]>
Cc: Thomas Munro <[email protected]>
Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
Date: Mon, 23 Nov 2020 18:00:31 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <alpine.DEB.2.21.2004121145190.5257@pseudo>
	<[email protected]>
	<[email protected]>
	<alpine.DEB.2.22.394.2006070908120.32228@pseudo>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Greetings,

* Tom Lane ([email protected]) wrote:
> Justin Pryzby <[email protected]> writes:
> >> This patch has been waiting for input from a committer on the approach I've
> >> taken with the patches since March 10, so I'm planning to set to "Ready" - at
> >> least ready for senior review.
> 
> I took a quick look through this.  This is just MHO, of course:
> 
> * I don't think it's okay to change the existing signatures of
> pg_ls_logdir() et al.  Even if you can make an argument that it's
> not too harmful to add more output columns, replacing pg_stat_file's
> isdir output with something of a different name and datatype is most
> surely not OK --- there is no possible way that doesn't break existing
> user queries.

I disagree that we need to stress over this- we pretty routinely change
the signature of various catalogs and functions and anyone using these
is already of the understanding that we are free to make such changes
between major versions.  If anything, we should be strongly discouraging
the notion of "don't break user queries" when it comes to administrative
and monitoring functions like these because, otherwise, we end up with
things like the mess that is pg_start/stop_backup() (and just contrast
that to what we did to recovery.conf when thinking about "well, do we
need to 'deprecate' or keep around the old stuff so we don't break
things for users who use these functions?" or the changes made in v10,
neither of which have produced much in the way of complaints).

Let's focus on working towards cleaner APIs and functions, accepting a
break when it makes sense to, which seems to be the case with this patch
(though I agree about using a TAP test suite and about performing the
directory recursion in C instead), and not pull forward cruft that we
then are telling ourselves we have to maintain compatibility of
indefinitely and at the expense of sensible APIs.

Thanks,

Stephen


Attachments:

  [application/pgp-signature] signature.asc (819B, ../[email protected]/2-signature.asc)
  download

view thread (57+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_ls_tmpdir to show directories and shared filesets (and pg_ls_*)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox