Dear Amit,

> 
> I think it would be better to write problematic slots in the script
> file like we are doing in the function
> check_for_composite_data_type_usage()->check_for_data_types_usage()
> and give a message suggesting what the user can do as we are doing in
> check_for_composite_data_type_usage(). That will be helpful for the
> user to take necessary action.

Did it. I wondered how we output the list of slots because there are two types 
of
problem, but currently I used a same file. If you have better approach, please
teach me.

> A few other comments:
> =================
> 1.
> @@ -189,6 +199,8 @@ check_new_cluster(void)
>  {
>   get_db_and_rel_infos(&new_cluster);
> 
> + check_new_cluster_logical_replication_slots();
> +
>   check_new_cluster_is_empty();
> 
>   check_loadable_libraries();
> 
> Why check_new_cluster_logical_replication_slots is done before
> check_new_cluster_is_empty? At least check_new_cluster_is_empty()
> would be much quicker to return an error if any. I think if we don't
> have a specific reason to position this new check, we can do it at the
> end after check_for_new_tablespace_dir() to avoid breaking the order
> of existing checks.

Moved to the bottom.

> 2. Shall we rename get_db_and_rel_infos() to
> get_db_rel_and_slot_infos() or something like that as that function
> now fetches the slot information as well?

Fixed. Comments were also fixed as well. 

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Reply via email to