From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Euler Taveira <euler(at)timbira(dot)com(dot)br>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: vacuumdb and new VACUUM options |
Date: | 2019-05-17 02:09:22 |
Message-ID: | 20190517020922.GJ20887@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, May 15, 2019 at 03:44:22PM +0900, Masahiko Sawada wrote:
> I've attached new version patch that takes the way to let the backend
> parser do all work.
I was wondering how the error handling gets by not having the parsing
on the frontend, and it could be worse:
$ vacuumdb --index-cleanup=popo
vacuumdb: vacuuming database "postgres"
vacuumdb: error: vacuuming of table "pg_catalog.pg_proc" in database
"postgres" failed: ERROR: index_cleanup requires a Boolean value
$ vacuumdb --truncate=popo
vacuumdb: vacuuming database "postgres"
vacuumdb: error: vacuuming of table "pg_catalog.pg_proc" in database
"postgres" failed: ERROR: truncate requires a Boolean value
For TRUNCATE, we actually get to the same error, and INDEX_CLEANUP
just defers with the separator between the two terms. I think that we
could live with that for simplicity's sake. Perhaps others have
different opinions though.
+ if (vacopts.index_cleanup != NULL)
Checking directly for NULL-ness here is inconsistent with the previous
callers.
+$node->issues_sql_like(
+ [ 'vacuumdb', '--index-cleanup=true', 'postgres' ],
+ qr/statement: VACUUM \(INDEX_CLEANUP true\).*;/,
+ 'vacuumdb --index-cleanup=true')
We should have a failure test here instead of testing two times the
same boolean parameter with opposite values to make sure that we still
complain on invalid input values.
+ Specify that <command>VACUUM</command> should attempt to remove
+ index entries pointing to dead tuples. If this option is not specified
+ the behavior depends on <literal>vacuum_index_cleanup</literal> option
+ for the table to be vacuumed.
The description of other commands do not mention directly VACUUM, and
have a more straight-forward description about the goal of the option.
So the first sentence could be reworked as follows:
Removes index entries pointing to dead tuples.
And the second for --truncate:
Truncates any empty pages at the end of the relation.
My 2c.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2019-05-17 02:21:35 | Re: PostgreSQL 12: Feature Highlights |
Previous Message | Peter Geoghegan | 2019-05-17 01:49:29 | Re: PostgreSQL 12: Feature Highlights |