On Mon, Oct 07, 2024 at 11:16:33AM -0500, Nathan Bossart wrote:
> Regarding commit 1ab67c9...
>
> On Wed, Sep 25, 2024 at 08:10:16AM +0900, Michael Paquier wrote:
> > Thanks. As I am kind of behind this one, I'll go fix it first. Let's
> > sort out the permission bits after that one is sorted out. REL_17_0
> > is out, so this can happen across all branches.
>
> For consistency with the surrounding code, I think we should schema-qualify
> the operator and add a newline after "WHERE relpersistence != 't'". If
> folks agree, I can handle committing the attached patch.
Not just code consistency. A code comment requires the schema qualification:
* Since we execute the constructed query with the default search_path
* (which could be unsafe), everything in this query MUST be fully
* qualified.
> --- a/src/bin/scripts/vacuumdb.c
> +++ b/src/bin/scripts/vacuumdb.c
> @@ -684,7 +684,8 @@ vacuum_one_database(ConnParams *cparams,
> * Exclude temporary tables, beginning the WHERE clause.
> */
> appendPQExpBufferStr(&catalog_query,
> - " WHERE c.relpersistence != " CppAsString2(RELPERSISTENCE_TEMP));
> + " WHERE c.relpersistence OPERATOR(pg_catalog.!=) "
> + CppAsString2(RELPERSISTENCE_TEMP) "\n");