On 1/22/19, 7:41 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> I have been looking at 0002, and I found a problem with the way
> ANALYZE queries are generated. For example take this table:
> CREATE TABLE aa1 (a int);
>
> Then if I try to run ANALYZE with vacuumdb it just works:
> $ vacuumdb -z --table 'aa1(b)'
> vacuumdb: vacuuming database "ioltas"
>
> Note that this fails with HEAD, but passes with your patch. The
> problem is that the query generated misses the lists of columns when
> processing them through split_table_columns_spec(), as what is
> generated is that:
> VACUUM (ANALYZE) public.aa1;
>
> So the result is actually incorrect because all the columns get
> processed.
>
> This patch moves the check about the existence of the relation when
> querying the catalogs, perhaps we would want the same for columns for
> consistency? Or not. That's a bit harder for sure, not impossible
> visibly, still that would mean duplicating a bunch of logic that the
> backend is doing by itself, so we could live without it I think.
Oh, wow. Thanks for pointing this out. I should have caught this.
With 0002, we are basically just throwing out the column lists
entirely as we obtain the qualified identifiers from the catalog
query. To fix this, I've added an optional CTE for tracking any
provided column lists. v5-0001 is your test patch for this case, and
v5-0002 splits out the work for split_table_columns_spec().
Nathan
