Re: Allowing REINDEX to have an optional name

Sorry, I meant to send this earlier..

On Sun, Jul 17, 2022 at 03:19:47PM +0900, Michael Paquier wrote:
> The second thing is test coverage. Using a REINDEX DATABASE/SYSTEM

> +my $catalog_toast_index = $node->safe_psql('postgres',
> + "SELECT indexrelid::regclass FROM pg_index WHERE indrelid = '$toast_table'::regclass;"
> +);
> +
> +# Set of SQL queries to cross-check the state of relfilenodes across
> +# REINDEX operations. A set of relfilenodes is saved from the catalogs
> +# and then compared with pg_class.
> +$node->safe_psql('postgres',
> + 'CREATE TABLE toast_relfilenodes (relname regclass, relfilenode oid);');

It looks like you named the table "toast_relfilenodes", but then also store
to it data for non-toast tables.

It's also a bit weird to call the column "relname" but use it to store the
::regclass. You later need to cast the column to text, so you may as well
store it as text, either relname or oid::regclass.

It seems like cluster.sql does this more succinctly.

-- Check that clustering sets new relfilenodes:
CREATE TEMP TABLE old_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ;
CLUSTER clstrpart USING clstrpart_idx;
CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ;
SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C";

> +# Save the relfilenode of a set of toast indexes, one from the catalog
> +# pg_constraint and one from the test table. This data is used for checks
> +# after some of the REINDEX operations done below, checking if they are
> +# changed.
> +my $fetch_toast_relfilenodes = qq{SELECT c.oid::regclass, c.relfilenode
> + FROM pg_class a
> + JOIN pg_class b ON (a.oid = b.reltoastrelid)
> + JOIN pg_index i on (a.oid = i.indrelid)
> + JOIN pg_class c on (i.indexrelid = c.oid)
> + WHERE b.oid IN ('pg_constraint'::regclass, 'test1'::regclass)};
> +# Same for relfilenodes of normal indexes. This saves the relfilenode
> +# from a catalog of pg_constraint, and the one from the test table.
> +my $fetch_index_relfilenodes = qq{SELECT oid, relfilenode
> + FROM pg_class
> + WHERE relname IN ('pg_constraint_oid_index', 'test1x')};
> +my $save_relfilenodes =
> + "INSERT INTO toast_relfilenodes $fetch_toast_relfilenodes;"
> + . "INSERT INTO toast_relfilenodes $fetch_index_relfilenodes;";
> +
> +# Query to compare a set of relfilenodes saved with the contents of pg_class.
> +# Note that this does not join using OIDs, as CONCURRENTLY would change them
> +# when reindexing. A filter is applied on the toast index names, even if this
> +# does not make a difference between the catalog and normal ones, the ordering
> +# based on the name is enough to ensure a fixed output.
> +my $compare_relfilenodes =
> + qq(SELECT regexp_replace(b.relname::text, '(pg_toast.pg_toast_)\\d{4,5}(_index)', '\\1<oid>\\2'),

Why {4,5} ?

> + CASE WHEN a.relfilenode = b.relfilenode THEN 'relfilenode is unchanged'
> + ELSE 'relfilenode has changed' END
> + FROM toast_relfilenodes b
> + JOIN pg_class a ON b.relname::text = a.oid::regclass::text
> + ORDER BY b.relname::text);
> +
> +# Save the set of relfilenodes and compare them.
> +$node->safe_psql('postgres', $save_relfilenodes);
> +$node->issues_sql_like(
> + [ 'reindexdb', 'postgres' ],
> + qr/statement: REINDEX DATABASE postgres;/,
> + 'SQL REINDEX run');
> +my $relnode_info = $node->safe_psql('postgres', $compare_relfilenodes);
> +is( $relnode_info, qq(pg_constraint_oid_index|relfilenode is unchanged
> +pg_toast.pg_toast_<oid>_index|relfilenode has changed
> +pg_toast.pg_toast_<oid>_index|relfilenode is unchanged
> +test1x|relfilenode has changed), 'relfilenode change after REINDEX DATABASE');
> +
> +# Re-save and run the second one.
> +$node->safe_psql('postgres',
> + "TRUNCATE toast_relfilenodes; $save_relfilenodes");
> +$node->issues_sql_like(
> + [ 'reindexdb', '-s', 'postgres' ],
> + qr/statement: REINDEX SYSTEM postgres;/,
> + 'reindex system tables');
> +$relnode_info = $node->safe_psql('postgres', $compare_relfilenodes);
> +is( $relnode_info, qq(pg_constraint_oid_index|relfilenode has changed
> +pg_toast.pg_toast_<oid>_index|relfilenode is unchanged
> +pg_toast.pg_toast_<oid>_index|relfilenode has changed
> +test1x|relfilenode is unchanged), 'relfilenode change after REINDEX SYSTEM');