Re: [Proposal] Allow users to specify multiple tables in VACUUM commands

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: "Bossart, Nathan" <bossartn(at)amazon(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: [Proposal] Allow users to specify multiple tables in VACUUM commands
Date: 2017-08-25 23:00:01
Message-ID: CA+TgmoZVXFRCrCXa5i1MMWdEd1xJgdpi7Qx7ONYZoyP8cKv8YQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 24, 2017 at 12:59 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> Robert, Amit and other folks working on extending the existing
> partitioning facility would be in better position to answer that, but
> I would think that we should have something as flexible as possible,
> and storing a list of relation OID in each VacuumRelation makes it
> harder to track the uniqueness of relations vacuumed. I agree that the
> concept of a partition with multiple parents induces a lot of
> problems. But the patch as proposed worries me as it complicates
> vacuum() with a double loop: one for each relation vacuumed, and one
> inside it with the list of OIDs. Modules calling vacuum() could also
> use flexibility, being able to analyze a custom list of columns for
> each relation has value as well.

So ... why have a double loop? I mean, you could just expand this out
to one entry per relation actually being vacuumed, couldn't you?

What happens if you say VACUUM partitioned_table (a), some_partition (b)?

+ oldcontext = MemoryContextSwitchTo(vac_context);
+ foreach(lc, relations)
+ temp_relations = lappend(temp_relations, copyObject(lfirst(lc)));
+ MemoryContextSwitchTo(oldcontext);
+ relations = temp_relations;

Can't we just copyObject() on the whole list?

- ListCell *cur;
-

Why change this? Generally, declaring a separate variable in an inner
scope seems like better style than reusing one that happens to be
lying around in the outer scope.

+ VacuumRelation *relinfo = (VacuumRelation *) lfirst(lc);

Could use lfirst_node.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-08-25 23:00:47 Re: Variable substitution in psql backtick expansion
Previous Message Tom Lane 2017-08-25 22:43:54 Re: Variable substitution in psql backtick expansion