On Thu, Feb 28, 2019 at 1:00 PM Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.com> wrote:
> From: Haribabu Kommi [mailto:kommi.harib...@gmail.com] > > Attached are the updated patches. > > Thanks, all look fixed. > > > > The target_server_type option yet to be implemented. > > Please let me review once more and proceed to testing when the above is > added, to make sure the final code looks good. I'd like to see how complex > the if conditions in multiple places would be after adding > target_server_type, and consider whether we can simplify them together with > you. Even now, the if conditions seem complicated to me... that's probably > due to the existence of read_write_host_index. > Yes, if checks are little bit complex because of additional checks to identify, I will check if there is any easier way to update them without introducing code duplication. While working on implementation of target_server_type new connection option for the libpq to specify master, slave and etc, there is no problem when the newly added target_server_type option is used separate, but when it is combined with the existing target_session_attrs, there may be some combinations that are not valid or such servers doesn't exist. Target_session_attrs Target_server_type read-write prefer-slave, slave prefer-read master, slave read-only master, prefer-slave I know that some of the cases above is possible, like master server with by default accepts read-only sessions. Instead of we put a check to validate what is right combination, how about allowing the combinations and in case if such combination is not possible, means there shouldn't be any server the supports the requirement, and connection fails. comments? And also as we need to support the new option to connect to servers < 12 also, this option sends the command "select pg_is_in_recovery()" to the server to find out whether the server is recovery mode or not? And also regarding the implementation point of view, the new target_server_type option validation is separately handled, means the check for the required server is handled in a separate switch case, when both options are given, first find out the required server for target_session_attrs and validate the same again with target_server_type? Regards, Haribabu Kommi Fujitsu Australia