"Bossart, Nathan" <bossartn@amazon.com> writes:
> [ 0001-vacuum_multiple_tables_v18.patch ]
I started to look at this, but soon became pretty desperately unhappy with
the way that it makes a bunch of tests on the relations and then releases
all locks on them. That either makes the tests pointless, or requires you
to invent completely bizarre semantics, like this:
* Check that all specified columns exist so that we can fast-fail * commands with multiple tables. If the column
disappearsbefore we * actually process it, we will emit a WARNING and skip it later on.
I think that the way this ought to work is you process the VacuumRelation
structs purely sequentially, each in its own transaction, so you don't
need leaps of faith about what to do if a relation changes between the
first time you look at it and when you actually come to process it.
The idea of "fast failing" because some later VacuumRelation struct might
contain an error seems like useless complication, both in terms of the
implementation and the user-visible behavior.
It looks like some of this stuff might be the fault of the
partitioned-tables patch more than your own changes, but no time like
the present to try to improve matters.
As for the other patch, I wonder if it might not be better to
silently ignore duplicate column names. But in either case, we don't
need a pre-check, IMO. I'd just leave it to the lookup loop in
do_analyze_rel to notice if it's looked up the same column number
twice. That would be more likely to lead to a back-patchable fix,
too. 0002 as it stands is useless as a back-patch basis, since it
proposes modifying code that doesn't even exist in the back branches.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers