Date: Fri, 15 Sep 2023 11:39:01 +0900 (JST)
Message-Id: <20230915.113901.1424588695581203565.horikyota.ntt@gmail.com>
To: nathandbossart@gmail.com
Cc: daniel@yesql.se, kuwamura@db.is.i.nagoya-u.ac.jp,
 pgsql-hackers@lists.postgresql.org
Subject: Re: bug fix and documentation improvement about vacuumdb
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20230914145757.GA1552775@nathanxps13>
References: <CAMyC8qp9mXPQd5D6s6CJxvmignsbTqGZwDDB6VYJOn1A8WG38w@mail.gmail.com>
	<9788CCDC-217A-48B0-A0F8-4F311F0F2022@yesql.se>
	<20230914145757.GA1552775@nathanxps13>
User-Agent: Mew version 6.8 on Emacs 27.2
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230915.113901.1424588695581203565.horikyota.ntt%40gmail.com>
Precedence: bulk

At Thu, 14 Sep 2023 07:57:57 -0700, Nathan Bossart <nathandbossart@gmail.com> wrote in 
> On Thu, Sep 14, 2023 at 02:06:51PM +0200, Daniel Gustafsson wrote:
> > I can reproduce that, a single -N works but adding multiple -N's makes none of
> > them excluded. The current coding does this:
> > 
> >     if (objfilter & OBJFILTER_SCHEMA_EXCLUDE)
> >         appendPQExpBufferStr(&catalog_query, "OPERATOR(pg_catalog.!=) ");
> > 
> > If the join is instead made to exclude the oids in listed_objects with a left
> > join and a clause on object_oid being null I can make the current query work
> > without adding a second clause.  I don't have strong feelings wrt if we should
> > add a NOT IN () or fix this JOIN, but we shouldn't have a faulty join together
> > with the fix. With your patch the existing join is left in place, let's fix that.
> 
> Yeah, I think we can fix the JOIN as you suggest.  I quickly put a patch
> together to demonstrate.  We should probably add some tests...

It seems to work fine. However, if we're aiming for consistent
spacing, the "IS NULL" (two spaces in between) might be an concern.

> >> Third, for the description of the -N option, I wonder if "vacuum all tables except 
> >> in the specified schema(s)" might be clearer. The current one says nothing about 
> >> tables not in the specified schema.
> > 
> > Maybe, but the point of vacuumdb is to analyze a database so I'm not sure who
> > would expect anything else than vacuuming everything but the excluded schema
> > when specifying -N.  What else could "vacuumdb -N foo" be interpreted to do
> > that can be confusing?
> 
> I agree with Daniel on this one.

+1.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center