On 8/23/17, 11:59 PM, "Michael Paquier" <michael.paquier@gmail.com> wrote:
> + * relations is a list of VacuumRelations to process. If it is NIL, all
> + * relations in the database are processed.
> Typo here, VacuumRelation is singular.
This should be fixed in v9.
On 8/24/17, 5:45 PM, "Michael Paquier" <michael.paquier@gmail.com> wrote:
> This makes me think that it could be a good idea to revisit this bit
> in a separate patch. ANALYZE fails as well now when the same column is
> defined multiple times with an incomprehensible error message.
The de-duplication code is now in a separate patch,
dedupe_vacuum_relations_v1.patch. I believe it fixes the incomprehensible
error message you were experiencing, but please let me know if you are
still hitting it.
On 8/25/17, 6:00 PM, "Robert Haas" <robertmhaas@gmail.com> wrote:
> + 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?
I've made this change.
> - 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.
I've removed this change.
> + VacuumRelation *relinfo = (VacuumRelation *) lfirst(lc);
>
> Could use lfirst_node.
Done.
On 8/28/17, 5:28 PM, "Michael Paquier" <michael.paquier@gmail.com> wrote:
> Yes, if I understand that correctly. That's the point I am exactly
> coming at. My suggestion is to have one VacuumRelation entry per
> relation vacuumed, even for partitioned tables, and copy the list of
> columns to each one.
I've made this change in v9. It does clean up the patch quite a bit.
Nathan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers