Re: bug fix and documentation improvement about vacuumdb

Thank you for all your reviews!

>>> PATTERN should be changed to SCHEMA because -n and -N options don't
support
>>> pattern matching for schema names. The attached patch 0001 fixes this.
>>
>> True, there is no pattern matching performed. I wonder if it's worth
lifting
>> the pattern matching from pg_dump into common code such that tools like
this
>> can use it?
>
> I agree that this should be changed to SCHEMA. It might be tough to add
> pattern matching with the current catalog query, and I don't know whether
> there is demand for such a feature, but I wouldn't discourage someone from
> trying.

I think that supporting pattern matching is quite nice.
But it will be not only tough but also a breaking change, I wonder.
So I guess this change should be commited either way.

>>> Yeah, I think we can fix the JOIN as you suggest. I quickly put a patch
>>> together to demonstrate.
>
> Looks good from a quick skim.

I do agree with this updates. Thank you!

>> We should probably add some tests...
>
> Agreed.

The attached patch includes new tests for this bug.
Also, I fixed the current test for -N option seems to be incorrect.

>>> It seems to work fine. However, if we're aiming for consistent
>>> spacing, the "IS NULL" (two spaces in between) might be an concern.
>>
>> I don't think that's a problem. I would rather have readable C code and
two
>> spaces in the generated SQL than contorting the C code to produce less
>> whitespace in a query few will read in its generated form.
>
> I think we could pretty easily avoid the extra space and keep the C code
> relatively readable. These sorts of things bug me, too (see 2af3336).

Though I don't think it affects readability, I'm neutral about this.

>> >> 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.

That make sense. I retract my suggestion.