On Wed, Jan 14, 2026 at 11:24 PM Masahiko Sawada <[email protected]> wrote: > > I've attached the updated patch. >
Thank You for the patch. I like the idea of optimization. Few initial comments: 1) + * The query returns the slot names and their caught-up status in + * the same order as the results collected by + * get_old_cluster_logical_slot_infos(). If this query is changed, I could not find the function get_old_cluster_logical_slot_infos(), do you mean get_old_cluster_logical_slot_infos_query()? 2) " WHERE database = current_database() AND " " slot_type = 'logical' AND " Is there a reason why database = current_database() is placed before slot_type = 'logical'? I am not sure how the PostgreSQL optimizer and executor will order these predicates, but from the first look, slot_type = 'logical' appears cheaper and could be placed first, consistent with the ordering used at other places. 3) Shouldn’t we add a sanity check inside get_old_cluster_logical_slot_infos_query() to ensure that when skip_caught_up_check is true, we are on PostgreSQL 18 or lower? This would make the function safer for future use if it's called elsewhere. I understand the caller already performs a similar check, but I think it's more appropriate here since we call binary_upgrade_logical_slot_has_caught_up() from inside, which doesn’t even exist on newer versions. 4) +# Check the file content. While both test_slot1 and test_slot2 should be reporting +# that they have unconsumed WAL records, test_slot3 should not be reported as +# it has caught up. Can you please elaborate the reason behind test_slot3 not being reported? Also mention in the comment if possible. thanks Shveta
