On Mon, Aug 21, 2023 at 6:32 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.com> wrote: > > > 2. > > + /* > > + * Checking for logical slots must be done before > > + * check_new_cluster_is_empty() because the slot_arr attribute of the > > + * new_cluster will be checked in that function. > > + */ > > + if (count_logical_slots(&old_cluster)) > > + { > > + get_logical_slot_infos(&new_cluster, false); > > + check_for_logical_replication_slots(&new_cluster); > > + } > > + > > check_new_cluster_is_empty(); > > > > Can't we simplify this checking by simply querying > > pg_replication_slots for any usable slot something similar to what we > > are doing in check_for_prepared_transactions()? We can add this check > > in the function check_for_logical_replication_slots(). > > Some checks were included to check_for_logical_replication_slots(), and > get_logical_slot_infos() for new_cluster was removed as you said. >
+ res = executeQueryOrDie(conn, "SELECT slot_name " + "FROM pg_catalog.pg_replication_slots " + "WHERE slot_type = 'logical' AND " + "temporary IS FALSE;"); + + if (PQntuples(res)) + pg_fatal("New cluster must not have logical replication slot, but found \"%s\"", + PQgetvalue(res, 0, 0)); + + PQclear(res); + + nslots = count_logical_slots(&old_cluster); + + /* + * Do additional checks when the logical replication slots have on the old + * cluster. + */ + if (nslots) Shouldn't these checks be reversed? I mean it would be better to test the presence of slots on the new cluster if there is any slot present on the old cluster. -- With Regards, Amit Kapila.